Changeset 192477 in webkit

Timestamp:

Nov 16, 2015 11:04:02 AM (8 years ago)

Author:

jiewen_tan@apple.com

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body-expected.txt (added)
• LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body.html (added)
• LayoutTests/editing/execCommand/justify-right-with-problematic-body-expected.txt (added)
• LayoutTests/editing/execCommand/justify-right-with-problematic-body.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/CompositeEditCommand.cpp (modified) (3 diffs)
• Source/WebCore/editing/htmlediting.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/editing/CompositeEditCommand.cpp

@@ -1013 +1013 @@
             }
         } 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.
@@ -1023 +1023 @@
     }
 
+    // If upstreamStart is not editable, then we can bail here.
+    if (!isEditablePosition(upstreamStart))
+        return nullptr;
     RefPtr<Node> newBlock = insertNewDefaultParagraphElementAt(upstreamStart);
 
@@ -1198 +1201 @@
 
     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());

trunk/Source/WebCore/editing/htmlediting.cpp

@@ -288 +288 @@
 Position firstEditablePositionAfterPositionInRoot(const Position& position, Node* highestRoot)
 {
+    if (!highestRoot)
+        return Position();
+
     // position falls before highestRoot.
     if (comparePositions(position, firstPositionInNode(highestRoot)) == -1 && highestRoot->hasEditableStyle())
@@ -313 +316 @@
 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)