Changeset 284792 in webkit

Timestamp:

Oct 25, 2021, 10:48:24 AM (5 years ago)

Author:

commit-queue@webkit.org

Message:

Source/WebCore:
ASSERT(node) triggered after surroundNodeRangeWithElement for node without editable style
​https://bugs.webkit.org/show_bug.cgi?id=232133

Patch by Gabriel Nava Marino <​gnavamarino@apple.com> on 2021-10-25
Reviewed by Wenson Hsieh.

If the last styled node was not parent node of a current text node, but we
wish to style the text node, we will add a style span to surround the text node.
However, this requires the parent to have an editable style, or
we will not properly insert the span in the right location, which
later leads to a traversal into an invalid node. This change
makes it so we return early if the parent node does not have an
editable style, but modifying the existing
CompositeEditCommand::insertNodeBefore to return a boolean in the
early return case.

Test: fast/editing/apply-relative-font-style-change-crash-003.html

• editing/ApplyStyleCommand.cpp:

(WebCore::ApplyStyleCommand::surroundNodeRangeWithElement):

• editing/CompositeEditCommand.cpp:

(WebCore::CompositeEditCommand::insertNodeBefore):

• editing/CompositeEditCommand.h:

LayoutTests:
ASSERT(node) triggered after surroundNodeRangeWithElement for node without editable style
​https://bugs.webkit.org/show_bug.cgi?id=232133

Patch by Gabriel Nava Marino <​gnavamarino@apple.com> on 2021-10-25
Reviewed by Wenson Hsieh.

• fast/editing/apply-relative-font-style-change-crash-003-expected.txt: Added.
• fast/editing/apply-relative-font-style-change-crash-003.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/editing/apply-relative-font-style-change-crash-003-expected.txt (added)
• LayoutTests/fast/editing/apply-relative-font-style-change-crash-003.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/ApplyStyleCommand.cpp (modified) (1 diff)
• Source/WebCore/editing/CompositeEditCommand.cpp (modified) (1 diff)
• Source/WebCore/editing/CompositeEditCommand.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-10-25  Gabriel Nava Marino  <gnavamarino@apple.com>
+
+        ASSERT(node) triggered after surroundNodeRangeWithElement for node without editable style 
+        https://bugs.webkit.org/show_bug.cgi?id=232133
+
+        Reviewed by Wenson Hsieh.
+
+        * fast/editing/apply-relative-font-style-change-crash-003-expected.txt: Added.
+        * fast/editing/apply-relative-font-style-change-crash-003.html: Added.
+
 2021-10-25  Ayumi Kojima  <ayumi_kojima@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-10-25  Gabriel Nava Marino  <gnavamarino@apple.com>
+
+        ASSERT(node) triggered after surroundNodeRangeWithElement for node without editable style
+        https://bugs.webkit.org/show_bug.cgi?id=232133
+
+        Reviewed by Wenson Hsieh.
+        
+        If the last styled node was not parent node of a current text node, but we 
+        wish to style the text node, we will add a style span to surround the text node.
+        However, this requires the parent to have an editable style, or
+        we will not properly insert the span in the right location, which
+        later leads to a traversal into an invalid node. This change
+        makes it so we return early if the parent node does not have an
+        editable style, but modifying the existing
+        CompositeEditCommand::insertNodeBefore to return a boolean in the
+        early return case.
+
+        Test: fast/editing/apply-relative-font-style-change-crash-003.html
+
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::ApplyStyleCommand::surroundNodeRangeWithElement):
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::insertNodeBefore):
+        * editing/CompositeEditCommand.h:
+
 2021-10-25  Darin Adler  <darin@apple.com>
 

trunk/Source/WebCore/editing/ApplyStyleCommand.cpp

@@ -1320 +1320 @@
     Ref<Element> element = WTFMove(elementToInsert);
 
-    insertNodeBefore(element.copyRef(), startNode);
-    if (!element->isContentRichlyEditable()) {
+    if (!insertNodeBefore(element.copyRef(), startNode) || !element->isContentRichlyEditable()) {
         removeNode(element);
         return false;

trunk/Source/WebCore/editing/CompositeEditCommand.cpp

@@ -553 +553 @@
 }
 
-void CompositeEditCommand::insertNodeBefore(Ref<Node>&& insertChild, Node& refChild, ShouldAssumeContentIsAlwaysEditable shouldAssumeContentIsAlwaysEditable)
+bool CompositeEditCommand::insertNodeBefore(Ref<Node>&& insertChild, Node& refChild, ShouldAssumeContentIsAlwaysEditable shouldAssumeContentIsAlwaysEditable)
 {
     RefPtr parent { refChild.parentNode() };
     if (!parent || (!parent->hasEditableStyle() && parent->renderer()))
-        return;
+        return false;
     applyCommandToComposite(InsertNodeBeforeCommand::create(WTFMove(insertChild), refChild, shouldAssumeContentIsAlwaysEditable, editingAction()));
+    return true;
 }
 

trunk/Source/WebCore/editing/CompositeEditCommand.h

@@ -155 +155 @@
     void insertNodeAt(Ref<Node>&&, const Position&);
     void insertNodeAtTabSpanPosition(Ref<Node>&&, const Position&);
-    void insertNodeBefore(Ref<Node>&&, Node& refChild, ShouldAssumeContentIsAlwaysEditable = DoNotAssumeContentIsAlwaysEditable);
+    bool insertNodeBefore(Ref<Node>&&, Node& refChild, ShouldAssumeContentIsAlwaysEditable = DoNotAssumeContentIsAlwaysEditable);
     void insertParagraphSeparatorAtPosition(const Position&, bool useDefaultParagraphElement = false, bool pasteBlockqutoeIntoUnquotedArea = false);
     void insertParagraphSeparator(bool useDefaultParagraphElement = false, bool pasteBlockqutoeIntoUnquotedArea = false);