Null-pointer dereference in WebCore::firstEditablePositionAfterPositionInRoot
authorjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Nov 2015 19:04:02 +0000 (19:04 +0000)
committerjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Nov 2015 19:04:02 +0000 (19:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=151288
<rdar://problem/23450367>

Reviewed by Darin Adler.

Source/WebCore:

Some problematic organization of body element could cause problems to JustifyRight
and Indent commnads.

Tests: editing/execCommand/justify-right-then-indent-with-problematic-body.html
       editing/execCommand/justify-right-with-problematic-body.html

* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary):
Assertion at l1017 is not held anymore with the testcase:
editing/execCommand/justify-right-with-problematic-body.html.
Therefore, change it to an if statement.
Also, add a guardance before calling insertNewDefaultParagraphElementAt()
as insertNodeAt() requires an editable position.
(WebCore::CompositeEditCommand::moveParagraphWithClones):
Add a guardance before calling insertNodeAt() as it requires an editable position.
* editing/htmlediting.cpp:
(WebCore::firstEditablePositionAfterPositionInRoot):
(WebCore::lastEditablePositionBeforePositionInRoot):

LayoutTests:

* editing/execCommand/justify-right-then-indent-with-problematic-body-expected.txt: Added.
* editing/execCommand/justify-right-then-indent-with-problematic-body.html: Added.
* editing/execCommand/justify-right-with-problematic-body-expected.txt: Added.
* editing/execCommand/justify-right-with-problematic-body.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body.html [new file with mode: 0644]
LayoutTests/editing/execCommand/justify-right-with-problematic-body-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/justify-right-with-problematic-body.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/CompositeEditCommand.cpp
Source/WebCore/editing/htmlediting.cpp

index 282a3514aafe55e11a86393332507decabd31efa..547ad70063a3a11ee967d81a33b59a176af4610d 100644 (file)
@@ -1,3 +1,16 @@
+2015-11-16  Jiewen Tan  <jiewen_tan@apple.com>
+
+        Null-pointer dereference in WebCore::firstEditablePositionAfterPositionInRoot
+        https://bugs.webkit.org/show_bug.cgi?id=151288
+        <rdar://problem/23450367>
+
+        Reviewed by Darin Adler.
+
+        * editing/execCommand/justify-right-then-indent-with-problematic-body-expected.txt: Added.
+        * editing/execCommand/justify-right-then-indent-with-problematic-body.html: Added.
+        * editing/execCommand/justify-right-with-problematic-body-expected.txt: Added.
+        * editing/execCommand/justify-right-with-problematic-body.html: Added.
+
 2015-11-16  Ryan Haddad  <ryanhaddad@apple.com>
 
         Rebaselining LayoutTest js/dom/global-constructors-attributes on Mac
diff --git a/LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body-expected.txt b/LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body-expected.txt
new file mode 100644 (file)
index 0000000..7f07c54
--- /dev/null
@@ -0,0 +1,3 @@
+Pass.
+WebKit didn't Crash.
+
diff --git a/LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body.html b/LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body.html
new file mode 100644 (file)
index 0000000..a637bca
--- /dev/null
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<body id="webtest3">
+Pass. <summary/>WebKit didn't Crash.<br>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+function editingTest() {
+    document.execCommand("SelectAll");
+    document.designMode = "on";
+    document.execCommand("JustifyRight", false, null);
+    document.execCommand("Indent", false, null);
+}
+editingTest();
+</script>
+</body>
+</html>
diff --git a/LayoutTests/editing/execCommand/justify-right-with-problematic-body-expected.txt b/LayoutTests/editing/execCommand/justify-right-with-problematic-body-expected.txt
new file mode 100644 (file)
index 0000000..af24a58
--- /dev/null
@@ -0,0 +1,4 @@
+Pass. WebKit didn't crash.
+
+
+
diff --git a/LayoutTests/editing/execCommand/justify-right-with-problematic-body.html b/LayoutTests/editing/execCommand/justify-right-with-problematic-body.html
new file mode 100644 (file)
index 0000000..ba47184
--- /dev/null
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<body>
+Pass. WebKit didn't crash.<ul><br><summary>
+<br><rt/>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+function editingTest() {
+    document.execCommand("SelectAll");
+    document.designMode = "on";
+    document.execCommand("JustifyRight", true, "Arial");
+}
+editingTest();
+</script>
+</body>
+</html>
\ No newline at end of file
index cdfe699b1b10d090b5422a82029e9687039a0063..f8485f115534b95b9607cc2d8d9a1841ff2c14c5 100644 (file)
@@ -1,3 +1,30 @@
+2015-11-16  Jiewen Tan  <jiewen_tan@apple.com>
+
+        Null-pointer dereference in WebCore::firstEditablePositionAfterPositionInRoot
+        https://bugs.webkit.org/show_bug.cgi?id=151288
+        <rdar://problem/23450367>
+
+        Reviewed by Darin Adler.
+
+        Some problematic organization of body element could cause problems to JustifyRight
+        and Indent commnads.
+
+        Tests: editing/execCommand/justify-right-then-indent-with-problematic-body.html
+               editing/execCommand/justify-right-with-problematic-body.html
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary):
+        Assertion at l1017 is not held anymore with the testcase:
+        editing/execCommand/justify-right-with-problematic-body.html.
+        Therefore, change it to an if statement.
+        Also, add a guardance before calling insertNewDefaultParagraphElementAt()
+        as insertNodeAt() requires an editable position.
+        (WebCore::CompositeEditCommand::moveParagraphWithClones):
+        Add a guardance before calling insertNodeAt() as it requires an editable position.
+        * editing/htmlediting.cpp:
+        (WebCore::firstEditablePositionAfterPositionInRoot):
+        (WebCore::lastEditablePositionBeforePositionInRoot):
+
 2015-11-16  Simon Fraser  <simon.fraser@apple.com>
 
         Sort the Xcode project files.
index 5440f08b8fcee21ac4c401da00cba349f94f1774..5e24ae210fb5802f9959151f3c5794254eab463c 100644 (file)
@@ -1012,16 +1012,19 @@ PassRefPtr<Node> CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessar
                 return nullptr;
             }
         } else if (enclosingBlock(upstreamEnd.deprecatedNode()) != upstreamStart.deprecatedNode()) {
-            // The visibleEnd.  It must be an ancestor of the paragraph start.
-            // We can bail as we have a full block to work with.
-            ASSERT(upstreamStart.deprecatedNode()->isDescendantOf(enclosingBlock(upstreamEnd.deprecatedNode())));
-            return nullptr;
+            // The visibleEnd. If it is an ancestor of the paragraph start, then
+            // we can bail as we have a full block to work with.
+            if (upstreamStart.deprecatedNode()->isDescendantOf(enclosingBlock(upstreamEnd.deprecatedNode())))
+                return nullptr;
         } else if (isEndOfEditableOrNonEditableContent(visibleEnd)) {
             // At the end of the editable region. We can bail here as well.
             return nullptr;
         }
     }
 
+    // If upstreamStart is not editable, then we can bail here.
+    if (!isEditablePosition(upstreamStart))
+        return nullptr;
     RefPtr<Node> newBlock = insertNewDefaultParagraphElementAt(upstreamStart);
 
     bool endWasBr = visibleParagraphEnd.deepEquivalent().deprecatedNode()->hasTagName(brTag);
@@ -1197,7 +1200,8 @@ void CompositeEditCommand::moveParagraphWithClones(const VisiblePosition& startO
     afterParagraph = VisiblePosition(afterParagraph.deepEquivalent());
 
     if (beforeParagraph.isNotNull() && !isRenderedTable(beforeParagraph.deepEquivalent().deprecatedNode())
-        && ((!isEndOfParagraph(beforeParagraph) && !isStartOfParagraph(beforeParagraph)) || beforeParagraph == afterParagraph)) {
+        && ((!isEndOfParagraph(beforeParagraph) && !isStartOfParagraph(beforeParagraph)) || beforeParagraph == afterParagraph)
+        && isEditablePosition(beforeParagraph.deepEquivalent())) {
         // FIXME: Trim text between beforeParagraph and afterParagraph if they aren't equal.
         insertNodeAt(createBreakElement(document()), beforeParagraph.deepEquivalent());
     }
index f949092518c3e66de763596aa12ed70127ace61f..306f4b2a5a20723f8b5292e2e77b474c04a2b06b 100644 (file)
@@ -287,6 +287,9 @@ Position previousVisuallyDistinctCandidate(const Position& position)
 
 Position firstEditablePositionAfterPositionInRoot(const Position& position, Node* highestRoot)
 {
+    if (!highestRoot)
+        return Position();
+
     // position falls before highestRoot.
     if (comparePositions(position, firstPositionInNode(highestRoot)) == -1 && highestRoot->hasEditableStyle())
         return firstPositionInNode(highestRoot);
@@ -312,6 +315,9 @@ Position firstEditablePositionAfterPositionInRoot(const Position& position, Node
 
 Position lastEditablePositionBeforePositionInRoot(const Position& position, Node* highestRoot)
 {
+    if (!highestRoot)
+        return Position();
+
     // When position falls after highestRoot, the result is easy to compute.
     if (comparePositions(position, lastPositionInNode(highestRoot)) == 1)
         return lastPositionInNode(highestRoot);