DeleteSelectionCommand should be robust when starting and ending editable positions...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Aug 2017 04:12:55 +0000 (04:12 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Aug 2017 04:12:55 +0000 (04:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=175914
<rdar://problem/29792688>

Reviewed by Ryosuke Niwa.

Source/WebCore:

DeleteSelectionCommand can cause a null dereference if editable start and end positions are not found. This can
happen when attempting to delete after selecting the contents within a canvas or output element with `read-write`
`-webkit-user-modify` style. To fix this, we make the initialization step of the DeleteSelectionCommand robust
when editable start and end positions are missing.

Test: editing/execCommand/forward-delete-read-write-canvas.html

* editing/DeleteSelectionCommand.cpp:
(WebCore::DeleteSelectionCommand::initializePositionData):

Make this initialization helper indicate failure via a bool return value. DeleteSelectionCommand::doApply bails
early if initializePositionData returned false.

(WebCore::DeleteSelectionCommand::doApply):
* editing/DeleteSelectionCommand.h:

LayoutTests:

Adds a new LayoutTest. This test passes if WebKit successfully loaded the page.

* editing/execCommand/forward-delete-read-write-canvas-expected.txt: Added.
* editing/execCommand/forward-delete-read-write-canvas.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@221128 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/execCommand/forward-delete-read-write-canvas-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/forward-delete-read-write-canvas.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/DeleteSelectionCommand.cpp
Source/WebCore/editing/DeleteSelectionCommand.h

index c36ad89..59d8020 100644 (file)
@@ -1,3 +1,16 @@
+2017-08-23  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        DeleteSelectionCommand should be robust when starting and ending editable positions cannot be found
+        https://bugs.webkit.org/show_bug.cgi?id=175914
+        <rdar://problem/29792688>
+
+        Reviewed by Ryosuke Niwa.
+
+        Adds a new LayoutTest. This test passes if WebKit successfully loaded the page.
+
+        * editing/execCommand/forward-delete-read-write-canvas-expected.txt: Added.
+        * editing/execCommand/forward-delete-read-write-canvas.html: Added.
+
 2017-08-23  Matt Lewis  <jlewis3@apple.com>
 
         Marked loader/stateobjects/replacestate-size.html as flaky.
diff --git a/LayoutTests/editing/execCommand/forward-delete-read-write-canvas-expected.txt b/LayoutTests/editing/execCommand/forward-delete-read-write-canvas-expected.txt
new file mode 100644 (file)
index 0000000..984ab80
--- /dev/null
@@ -0,0 +1 @@
+PASS 
diff --git a/LayoutTests/editing/execCommand/forward-delete-read-write-canvas.html b/LayoutTests/editing/execCommand/forward-delete-read-write-canvas.html
new file mode 100644 (file)
index 0000000..b6083b3
--- /dev/null
@@ -0,0 +1,9 @@
+<code style="color: green">PASS</code>
+<canvas style="-webkit-user-modify: read-write"><span><input id="input"></input></span></canvas>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+input.focus();
+input.remove();
+document.execCommand("ForwardDelete");
+</script>
index 0da65b5..96e1a52 100644 (file)
@@ -1,3 +1,27 @@
+2017-08-23  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        DeleteSelectionCommand should be robust when starting and ending editable positions cannot be found
+        https://bugs.webkit.org/show_bug.cgi?id=175914
+        <rdar://problem/29792688>
+
+        Reviewed by Ryosuke Niwa.
+
+        DeleteSelectionCommand can cause a null dereference if editable start and end positions are not found. This can
+        happen when attempting to delete after selecting the contents within a canvas or output element with `read-write`
+        `-webkit-user-modify` style. To fix this, we make the initialization step of the DeleteSelectionCommand robust
+        when editable start and end positions are missing.
+
+        Test: editing/execCommand/forward-delete-read-write-canvas.html
+
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::initializePositionData):
+
+        Make this initialization helper indicate failure via a bool return value. DeleteSelectionCommand::doApply bails
+        early if initializePositionData returned false.
+
+        (WebCore::DeleteSelectionCommand::doApply):
+        * editing/DeleteSelectionCommand.h:
+
 2017-08-23  Youenn Fablet  <youenn@apple.com>
 
         [Cache API] Unify WebCore and WebKit error handling
index a705be1..fa97a33 100644 (file)
@@ -172,7 +172,7 @@ void DeleteSelectionCommand::setStartingSelectionOnSmartDelete(const Position& s
     setStartingSelection(VisibleSelection(newBase, newExtent, startingSelection().isDirectional())); 
 }
     
-void DeleteSelectionCommand::initializePositionData()
+bool DeleteSelectionCommand::initializePositionData()
 {
     Position start, end;
     initializeStartEnd(start, end);
@@ -182,6 +182,9 @@ void DeleteSelectionCommand::initializePositionData()
     if (!isEditablePosition(end, ContentIsEditable))
         end = lastEditablePositionBeforePositionInRoot(end, highestEditableRoot(start));
 
+    if (start.isNull() || end.isNull())
+        return false;
+
     m_upstreamStart = start.upstream();
     m_downstreamStart = start.downstream();
     m_upstreamEnd = end.upstream();
@@ -272,6 +275,8 @@ void DeleteSelectionCommand::initializePositionData()
     // node.  This was done to match existing behavior, but it seems wrong.
     m_startBlock = enclosingNodeOfType(m_downstreamStart.parentAnchoredEquivalent(), &isBlock, CanCrossEditingBoundary);
     m_endBlock = enclosingNodeOfType(m_upstreamEnd.parentAnchoredEquivalent(), &isBlock, CanCrossEditingBoundary);
+
+    return true;
 }
 
 void DeleteSelectionCommand::saveTypingStyleState()
@@ -857,7 +862,8 @@ void DeleteSelectionCommand::doApply()
         
     
     // set up our state
-    initializePositionData();
+    if (!initializePositionData())
+        return;
 
     // Delete any text that may hinder our ability to fixup whitespace after the delete
     deleteInsignificantTextDownstream(m_trailingWhitespace);    
index 296ad9e..82cfe2a 100644 (file)
@@ -54,7 +54,7 @@ private:
 
     void initializeStartEnd(Position&, Position&);
     void setStartingSelectionOnSmartDelete(const Position&, const Position&);
-    void initializePositionData();
+    bool initializePositionData();
     void saveTypingStyleState();
     void insertPlaceholderForAncestorBlockContent();
     bool handleSpecialCaseBRDelete();