Changeset 269946 in webkit

Timestamp:

Nov 18, 2020 12:28:02 AM (3 years ago)

Author:

commit-queue@webkit.org

Message:

Release assertion failure in Optional<WebCore::SimpleRange>::operator* via CompositeEditCommand::moveParagraphs
​https://bugs.webkit.org/show_bug.cgi?id=218494

Patch by Carlos Garcia Campos <​cgarcia@igalia.com> on 2020-11-18
Reviewed by Ryosuke Niwa.

Source/WebCore:

This is happening when insert list command is called for a list item containing a body element as a child. When
the tree is iterated looking for the end position, the body element selected as candidate, but a null position
is returned because it's considered to be in a different editing element. This happens because
Node::rootEditableElement() always returns the node itseld for body elements, but it should actually check that
the node is the document body.

Test: editing/inserting/insert-list-with-body-child-crash.html

• dom/Node.cpp:

(WebCore::Node::isRootEditableElement const): Check node is the document body, not just a body element.
(WebCore::Node::rootEditableElement const): Ditto.

• editing/CompositeEditCommand.cpp:

(WebCore::CompositeEditCommand::moveParagraphs): Add an assert to ensure it's not called with a null endOfParagraphToMove.

LayoutTests:

• editing/inserting/insert-list-with-body-child-crash-expected.txt: Added.
• editing/inserting/insert-list-with-body-child-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/inserting/insert-list-with-body-child-crash-expected.txt (added)
• LayoutTests/editing/inserting/insert-list-with-body-child-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Node.cpp (modified) (2 diffs)
• Source/WebCore/editing/CompositeEditCommand.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-11-18  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        Release assertion failure in Optional<WebCore::SimpleRange>::operator* via CompositeEditCommand::moveParagraphs
+        https://bugs.webkit.org/show_bug.cgi?id=218494
+
+        Reviewed by Ryosuke Niwa.
+
+        * editing/inserting/insert-list-with-body-child-crash-expected.txt: Added.
+        * editing/inserting/insert-list-with-body-child-crash.html: Added.
+
 2020-11-17  Lauro Moura  <lmoura@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-11-18  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        Release assertion failure in Optional<WebCore::SimpleRange>::operator* via CompositeEditCommand::moveParagraphs
+        https://bugs.webkit.org/show_bug.cgi?id=218494
+
+        Reviewed by Ryosuke Niwa.
+
+        This is happening when insert list command is called for a list item containing a body element as a child. When
+        the tree is iterated looking for the end position, the body element selected as candidate, but a null position
+        is returned because it's considered to be in a different editing element. This happens because
+        Node::rootEditableElement() always returns the node itseld for body elements, but it should actually check that
+        the node is the document body.
+
+        Test: editing/inserting/insert-list-with-body-child-crash.html
+
+        * dom/Node.cpp:
+        (WebCore::Node::isRootEditableElement const): Check node is the document body, not just a body element.
+        (WebCore::Node::rootEditableElement const): Ditto.
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::moveParagraphs): Add an assert to ensure it's not called with a null endOfParagraphToMove.
+
 2020-11-17  Said Abou-Hallawa  <said@apple.com>
 

trunk/Source/WebCore/dom/Node.cpp

@@ -1328 +1328 @@
 {
     return hasEditableStyle() && isElementNode() && (!parentNode() || !parentNode()->hasEditableStyle()
-        || !parentNode()->isElementNode() || hasTagName(bodyTag));
+        || !parentNode()->isElementNode() || document().body() == this);
 }
 
@@ -1337 +1337 @@
         if (is<Element>(*node))
             result = downcast<Element>(node);
-        if (is<HTMLBodyElement>(*node))
+        if (document().body() == node)
             break;
     }

trunk/Source/WebCore/editing/CompositeEditCommand.cpp

@@ -1401 +1401 @@
         return;
 
+    ASSERT((startOfParagraphToMove.isNull() && endOfParagraphToMove.isNull()) || !endOfParagraphToMove.isNull());
+
     Optional<uint64_t> startIndex;
     Optional<uint64_t> endIndex;