Title: [276558] trunk
Revision
276558
Author
commit-qu...@webkit.org
Date
2021-04-24 15:31:57 -0700 (Sat, 24 Apr 2021)

Log Message

Crash in BreakBlockquoteCommand::doApply()
https://bugs.webkit.org/show_bug.cgi?id=224941

Patch by Julian Gonzalez <julian_a_gonza...@apple.com> on 2021-04-24
Reviewed by Ryosuke Niwa.

Source/WebCore:

Despite assertions to the contrary, it is possible for there not to be any node
to move into the new blockquote in BreakBlockquoteCommand::doApply() as a result
of layout updates, so remove the assertions and handle this case.

Test: editing/pasteboard/paste-as-quotation-then-paste-crash.html

* editing/BreakBlockquoteCommand.cpp:
(WebCore::BreakBlockquoteCommand::doApply):

LayoutTests:

Add test for this crash, running only on Release for now.
Thanks to Tuomas Karkkainen for its basic structure.

* TestExpectations:
* editing/pasteboard/paste-as-quotation-then-paste-crash-expected.txt: Added.
* editing/pasteboard/paste-as-quotation-then-paste-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (276557 => 276558)


--- trunk/LayoutTests/ChangeLog	2021-04-24 21:29:21 UTC (rev 276557)
+++ trunk/LayoutTests/ChangeLog	2021-04-24 22:31:57 UTC (rev 276558)
@@ -1,3 +1,17 @@
+2021-04-24  Julian Gonzalez  <julian_a_gonza...@apple.com>
+
+        Crash in BreakBlockquoteCommand::doApply()
+        https://bugs.webkit.org/show_bug.cgi?id=224941
+
+        Reviewed by Ryosuke Niwa.
+
+        Add test for this crash, running only on Release for now.
+        Thanks to Tuomas Karkkainen for its basic structure.
+
+        * TestExpectations:
+        * editing/pasteboard/paste-as-quotation-then-paste-crash-expected.txt: Added.
+        * editing/pasteboard/paste-as-quotation-then-paste-crash.html: Added.
+
 2021-04-24  Zalan Bujtas  <za...@apple.com>
 
         [RenderTreeBuilder] Subtree moving should clear the floats on all the descendants

Modified: trunk/LayoutTests/TestExpectations (276557 => 276558)


--- trunk/LayoutTests/TestExpectations	2021-04-24 21:29:21 UTC (rev 276557)
+++ trunk/LayoutTests/TestExpectations	2021-04-24 22:31:57 UTC (rev 276558)
@@ -1745,6 +1745,8 @@
 webkit.org/b/139634 [ Debug ] fast/selectors/nth-child-of-register-requirement.html [ Slow ]
 webkit.org/b/139634 [ Debug ] fast/selectors/not-backtracking.html [ Slow ]
 
+webkit.org/b/224941 [ Debug ] editing/pasteboard/paste-as-quotation-then-paste-crash.html [ WontFix ]
+
 webkit.org/b/61932 [ Debug ] jquery/manipulation.html [ Slow ]
 [ Debug ] jquery/core.html [ Slow ]
 [ Debug ] jquery/event.html [ Slow ]

Added: trunk/LayoutTests/editing/pasteboard/paste-as-quotation-then-paste-crash-expected.txt (0 => 276558)


--- trunk/LayoutTests/editing/pasteboard/paste-as-quotation-then-paste-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/paste-as-quotation-then-paste-crash-expected.txt	2021-04-24 22:31:57 UTC (rev 276558)
@@ -0,0 +1 @@
+This test passes if WebKit does not crash. PASS

Added: trunk/LayoutTests/editing/pasteboard/paste-as-quotation-then-paste-crash.html (0 => 276558)


--- trunk/LayoutTests/editing/pasteboard/paste-as-quotation-then-paste-crash.html	                        (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/paste-as-quotation-then-paste-crash.html	2021-04-24 22:31:57 UTC (rev 276558)
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+:last-child {
+    all: initial;
+}
+</style>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+_onload_ = () => {
+    let n0 = document.createElement('div');
+    document.documentElement.appendChild(n0);
+    n0.appendChild(document.createElement('input'));
+    let n1 = document.createElement('input');
+    document.documentElement.appendChild(n1);
+    getSelection().selectAllChildren(n0);
+    document.execCommand('Copy');
+    document.designMode = 'on';
+    document.execCommand('PasteAsQuotation');
+    getSelection().extend(n1);
+    document.execCommand('Paste');
+};
+</script>
+</head>
+<body>
+This test passes if WebKit does not crash. PASS
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (276557 => 276558)


--- trunk/Source/WebCore/ChangeLog	2021-04-24 21:29:21 UTC (rev 276557)
+++ trunk/Source/WebCore/ChangeLog	2021-04-24 22:31:57 UTC (rev 276558)
@@ -1,3 +1,19 @@
+2021-04-24  Julian Gonzalez  <julian_a_gonza...@apple.com>
+
+        Crash in BreakBlockquoteCommand::doApply()
+        https://bugs.webkit.org/show_bug.cgi?id=224941
+
+        Reviewed by Ryosuke Niwa.
+
+        Despite assertions to the contrary, it is possible for there not to be any node
+        to move into the new blockquote in BreakBlockquoteCommand::doApply() as a result
+        of layout updates, so remove the assertions and handle this case.
+
+        Test: editing/pasteboard/paste-as-quotation-then-paste-crash.html
+
+        * editing/BreakBlockquoteCommand.cpp:
+        (WebCore::BreakBlockquoteCommand::doApply):
+
 2021-04-24  Antoine Quint  <grao...@webkit.org>
 
         Improve parsing and computed style of the rotate CSS property

Modified: trunk/Source/WebCore/editing/BreakBlockquoteCommand.cpp (276557 => 276558)


--- trunk/Source/WebCore/editing/BreakBlockquoteCommand.cpp	2021-04-24 21:29:21 UTC (rev 276557)
+++ trunk/Source/WebCore/editing/BreakBlockquoteCommand.cpp	2021-04-24 22:31:57 UTC (rev 276558)
@@ -109,14 +109,15 @@
     if (is<Text>(*startNode)) {
         Text& textNode = downcast<Text>(*startNode);
         if ((unsigned)pos.deprecatedEditingOffset() >= textNode.length()) {
-            startNode = NodeTraversal::next(*startNode);
-            ASSERT(startNode);
+            if (auto* nextNode = NodeTraversal::next(*startNode))
+                startNode = nextNode;
         } else if (pos.deprecatedEditingOffset() > 0)
             splitTextNode(textNode, pos.deprecatedEditingOffset());
     } else if (pos.deprecatedEditingOffset() > 0) {
-        Node* childAtOffset = startNode->traverseToChildAt(pos.deprecatedEditingOffset());
-        startNode = childAtOffset ? childAtOffset : NodeTraversal::next(*startNode);
-        ASSERT(startNode);
+        if (auto* child = startNode->traverseToChildAt(pos.deprecatedEditingOffset()))
+            startNode = child;
+        else if (auto* next = NodeTraversal::next(*startNode))
+            startNode = next;
     }
     
     // If there's nothing inside topBlockquote to move, we're finished.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to