Title: [192477] trunk
Revision
192477
Author
jiewen_...@apple.com
Date
2015-11-16 11:04:02 -0800 (Mon, 16 Nov 2015)

Log Message

Null-pointer dereference in WebCore::firstEditablePositionAfterPositionInRoot
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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (192476 => 192477)


--- trunk/LayoutTests/ChangeLog	2015-11-16 18:50:45 UTC (rev 192476)
+++ trunk/LayoutTests/ChangeLog	2015-11-16 19:04:02 UTC (rev 192477)
@@ -1,3 +1,16 @@
+2015-11-16  Jiewen Tan  <jiewen_...@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  <ryanhad...@apple.com>
 
         Rebaselining LayoutTest js/dom/global-constructors-attributes on Mac

Added: trunk/LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body-expected.txt (0 => 192477)


--- trunk/LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body-expected.txt	2015-11-16 19:04:02 UTC (rev 192477)
@@ -0,0 +1,3 @@
+Pass.
+WebKit didn't Crash.
+

Added: trunk/LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body.html (0 => 192477)


--- trunk/LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body.html	                        (rev 0)
+++ trunk/LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body.html	2015-11-16 19:04:02 UTC (rev 192477)
@@ -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>

Added: trunk/LayoutTests/editing/execCommand/justify-right-with-problematic-body-expected.txt (0 => 192477)


--- trunk/LayoutTests/editing/execCommand/justify-right-with-problematic-body-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/execCommand/justify-right-with-problematic-body-expected.txt	2015-11-16 19:04:02 UTC (rev 192477)
@@ -0,0 +1,4 @@
+Pass. WebKit didn't crash.
+
+
+

Added: trunk/LayoutTests/editing/execCommand/justify-right-with-problematic-body.html (0 => 192477)


--- trunk/LayoutTests/editing/execCommand/justify-right-with-problematic-body.html	                        (rev 0)
+++ trunk/LayoutTests/editing/execCommand/justify-right-with-problematic-body.html	2015-11-16 19:04:02 UTC (rev 192477)
@@ -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

Modified: trunk/Source/WebCore/ChangeLog (192476 => 192477)


--- trunk/Source/WebCore/ChangeLog	2015-11-16 18:50:45 UTC (rev 192476)
+++ trunk/Source/WebCore/ChangeLog	2015-11-16 19:04:02 UTC (rev 192477)
@@ -1,3 +1,30 @@
+2015-11-16  Jiewen Tan  <jiewen_...@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.fra...@apple.com>
 
         Sort the Xcode project files.

Modified: trunk/Source/WebCore/editing/CompositeEditCommand.cpp (192476 => 192477)


--- trunk/Source/WebCore/editing/CompositeEditCommand.cpp	2015-11-16 18:50:45 UTC (rev 192476)
+++ trunk/Source/WebCore/editing/CompositeEditCommand.cpp	2015-11-16 19:04:02 UTC (rev 192477)
@@ -1012,16 +1012,19 @@
                 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 @@
     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());
     }

Modified: trunk/Source/WebCore/editing/htmlediting.cpp (192476 => 192477)


--- trunk/Source/WebCore/editing/htmlediting.cpp	2015-11-16 18:50:45 UTC (rev 192476)
+++ trunk/Source/WebCore/editing/htmlediting.cpp	2015-11-16 19:04:02 UTC (rev 192477)
@@ -287,6 +287,9 @@
 
 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 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);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to