REGRESSION (r236785): Nullptr crash in StyledMarkupAccumulator::traverseNodesForSeria...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 24 Nov 2018 02:17:30 +0000 (02:17 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 24 Nov 2018 02:17:30 +0000 (02:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191921

Reviewed by Dean Jackson.

Source/WebCore:

The bug was caused by traverseNodesForSerialization not being able to traverse past the end of shadow root
when skipping children of a node for which enterNode returns false because  it was using NodeTraversal's
nextSkippingChildren instead of a member function which supports traversing the composed tree.

Fixed the crash by using variant of nextSkippingChildren which knows how to traverse past the last node
in a shadow tree. Also added more assertions to help debug issues like this in the future.

Test: editing/pasteboard/copy-paste-across-shadow-boundaries-5.html

* editing/markup.cpp:
(WebCore::StyledMarkupAccumulator::traverseNodesForSerialization):

LayoutTests:

Added a regression test.

* editing/pasteboard/copy-paste-across-shadow-boundaries-5-expected.txt: Added.
* editing/pasteboard/copy-paste-across-shadow-boundaries-5.html: Added.
* platform/ios/editing/pasteboard/copy-paste-across-shadow-boundaries-5-expected.txt: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238465 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/copy-paste-across-shadow-boundaries-5-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/copy-paste-across-shadow-boundaries-5.html [new file with mode: 0644]
LayoutTests/platform/ios/editing/pasteboard/copy-paste-across-shadow-boundaries-5-expected.txt [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/markup.cpp

index 53849e2..f64e0b4 100644 (file)
@@ -1,3 +1,16 @@
+2018-11-23  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION (r236785): Nullptr crash in StyledMarkupAccumulator::traverseNodesForSerialization
+        https://bugs.webkit.org/show_bug.cgi?id=191921
+
+        Reviewed by Dean Jackson.
+
+        Added a regression test.
+
+        * editing/pasteboard/copy-paste-across-shadow-boundaries-5-expected.txt: Added.
+        * editing/pasteboard/copy-paste-across-shadow-boundaries-5.html: Added.
+        * platform/ios/editing/pasteboard/copy-paste-across-shadow-boundaries-5-expected.txt: Added.
+
 2018-11-22  Ryosuke Niwa  <rniwa@webkit.org>
 
         Updating href on textPath doesn't update its rendering
diff --git a/LayoutTests/editing/pasteboard/copy-paste-across-shadow-boundaries-5-expected.txt b/LayoutTests/editing/pasteboard/copy-paste-across-shadow-boundaries-5-expected.txt
new file mode 100644 (file)
index 0000000..90c7a5e
--- /dev/null
@@ -0,0 +1,13 @@
+This tests copying and pasting content across shadow boundaries.
+To test manually, copy text below, and paste into the green box below. All the text shoul be copied & pasted.
+
+pasted html:
+| <span>
+|   id="host"
+|   <span>
+|     style="display: contents;"
+|     "world"
+| "¬†WebKit<#selection-caret>"
+
+text/plain:
+| "world WebKit"
diff --git a/LayoutTests/editing/pasteboard/copy-paste-across-shadow-boundaries-5.html b/LayoutTests/editing/pasteboard/copy-paste-across-shadow-boundaries-5.html
new file mode 100644 (file)
index 0000000..c5598be
--- /dev/null
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p id="description">This tests copying and pasting content across shadow boundaries.<br>
+To test manually, copy text below, and paste into the green box below. All the text shoul be copied & pasted.</p>
+<style> .box { border: solid 1px; padding: 0.2rem; margin-bottom: 1rem; } </style>
+<div id="source" class="box" style="border-color: blue"><span id="host">world</span> WebKit</div>
+<div id="destination" class="box" style="border-color: blue" contenteditable></div>
+<pre id="htmlResult"></pre>
+<pre id="textResult"></pre>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+
+Markup.description(description.textContent);
+Markup.waitUntilDone();
+
+const shadowRoot = host.attachShadow({mode: 'open'});
+shadowRoot.innerHTML = 'hello <slot></slot><span style="display:none;"></span>';
+
+destination.addEventListener('paste', (event) => {
+    htmlResult.textContent = event.clipboardData.getData('text/html');
+    textResult.textContent = event.clipboardData.getData('text/plain');
+    setTimeout(() => {
+        Markup.dump(destination, 'pasted html');
+        Markup.dump(textResult, 'text/plain');
+        Markup.notifyDone();
+    }, 0);
+});
+
+if (window.testRunner) {
+    internals.setSelectionWithoutValidation(source, 0, source, source.childNodes.length);
+    document.execCommand('copy');
+    destination.focus();
+    document.execCommand('paste');
+}
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/platform/ios/editing/pasteboard/copy-paste-across-shadow-boundaries-5-expected.txt b/LayoutTests/platform/ios/editing/pasteboard/copy-paste-across-shadow-boundaries-5-expected.txt
new file mode 100644 (file)
index 0000000..6d66994
--- /dev/null
@@ -0,0 +1,16 @@
+This tests copying and pasting content across shadow boundaries.
+To test manually, copy text below, and paste into the green box below. All the text shoul be copied & pasted.
+
+pasted html:
+| <span>
+|   id="host"
+|   style="-webkit-text-size-adjust: auto;"
+|   <span>
+|     style="display: contents;"
+|     "world"
+| <span>
+|   style="-webkit-text-size-adjust: auto;"
+|   "¬†WebKit<#selection-caret>"
+
+text/plain:
+| "world WebKit"
index 70faf9a..ffa71c3 100644 (file)
@@ -1,3 +1,22 @@
+2018-11-23  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION (r236785): Nullptr crash in StyledMarkupAccumulator::traverseNodesForSerialization
+        https://bugs.webkit.org/show_bug.cgi?id=191921
+
+        Reviewed by Dean Jackson.
+
+        The bug was caused by traverseNodesForSerialization not being able to traverse past the end of shadow root
+        when skipping children of a node for which enterNode returns false because  it was using NodeTraversal's
+        nextSkippingChildren instead of a member function which supports traversing the composed tree.
+
+        Fixed the crash by using variant of nextSkippingChildren which knows how to traverse past the last node
+        in a shadow tree. Also added more assertions to help debug issues like this in the future.
+
+        Test: editing/pasteboard/copy-paste-across-shadow-boundaries-5.html
+
+        * editing/markup.cpp:
+        (WebCore::StyledMarkupAccumulator::traverseNodesForSerialization):
+
 2018-11-22  Ryosuke Niwa  <rniwa@webkit.org>
 
         Updating href on textPath doesn't update its rendering
index ffd74db..b7b2475 100644 (file)
@@ -645,6 +645,7 @@ Node* StyledMarkupAccumulator::traverseNodesForSerialization(Node* startNode, No
                 }
             }
         }
+        ASSERT(next || !pastEnd);
 
         if (isBlock(n) && canHaveChildrenForEditing(*n) && next == pastEnd) {
             // Don't write out empty block containers that aren't fully selected.
@@ -652,10 +653,11 @@ Node* StyledMarkupAccumulator::traverseNodesForSerialization(Node* startNode, No
         }
 
         if (!enterNode(*n)) {
-            next = NodeTraversal::nextSkippingChildren(*n);
+            next = nextSkippingChildren(*n);
             // Don't skip over pastEnd.
             if (pastEnd && isDescendantOf(*pastEnd, *n))
                 next = pastEnd;
+            ASSERT(next || !pastEnd);
         } else {
             if (!hasChildNodes(*n))
                 exitNode(*n);