-webkit-line-break: after-white-space sometimes truncates DOM on copy…

… & paste https://bugs.webkit.org/show_bug.cgi?id=118164 Reviewed by Sam Weinig. Source/WebCore: We can't assume that all subsequent ancestors contain exactly one child since they could have been added in the first if statement matching: currentNode->parentNode() != rootNode && isRemovableBlock(currentNode) Exit early when we encounter such an ancestor since removing its ancestor (that contains multiple children some of which aren't in nodesToRemove) can clobber more nodes than we're allowed to remove. Test: editing/pasteboard/simplfiying-markup-should-not-strip-content.html * editing/SimplifyMarkupCommand.cpp: (WebCore::SimplifyMarkupCommand::doApply): (WebCore::SimplifyMarkupCommand::pruneSubsequentAncestorsToRemove): LayoutTests: Add a regression test. * editing/pasteboard/simplfiying-markup-should-not-strip-content-expected.txt: Added. * editing/pasteboard/simplfiying-markup-should-not-strip-content.html: Added. Canonical link: https://commits.webkit.org/136239@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@152185 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Jun 28, 2013 · 2792c6f · 2792c6f
1 parent d6c6920
commit 2792c6f
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 1 deletion.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2013-06-28  Ryosuke Niwa  <rniwa@webkit.org>
+
+        -webkit-line-break: after-white-space sometimes truncates DOM on copy & paste
+        https://bugs.webkit.org/show_bug.cgi?id=118164
+
+        Reviewed by Sam Weinig.
+
+        Add a regression test.
+
+        * editing/pasteboard/simplfiying-markup-should-not-strip-content-expected.txt: Added.
+        * editing/pasteboard/simplfiying-markup-should-not-strip-content.html: Added.
+
 2013-06-28  Jer Noble  <jer.noble@apple.com>
 
         media/video-currentTime.html flakey

diff --git a/LayoutTests/editing/pasteboard/simplfiying-markup-should-not-strip-content-expected.txt b/LayoutTests/editing/pasteboard/simplfiying-markup-should-not-strip-content-expected.txt
@@ -0,0 +1,39 @@
+This tests copying and pasting doesn't strip content.
+To manually test, copy the content in the first box and paste it into the second box.
+
+Original content:
+| "
+"
+| <font>
+|   face="Verdana"
+|   "hello "
+| <br>
+| "
+"
+| <font>
+|   face="Verdana"
+|   <div>
+|     style="-webkit-line-break: after-white-space;"
+|     <div>
+|       <font>
+|         face="Verdana"
+|         "world"
+|     "
+"
+|     <div>
+|       style="-webkit-line-break: after-white-space; "
+|       <font>
+|         face="Verdana"
+|         "WebKit"
+|   "
+"
+
+Pasted content:
+| <font>
+|   face="Verdana"
+|   "hello "
+| <br>
+|   style="font-family: Helvetica;"
+| <font>
+|   face="Verdana"
+|   "worldWebKit<#selection-caret>"
diff --git a/LayoutTests/editing/pasteboard/simplfiying-markup-should-not-strip-content.html b/LayoutTests/editing/pasteboard/simplfiying-markup-should-not-strip-content.html
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html>
+<body style="font-family: Helvetica">
+<p id="description">This tests copying and pasting doesn't strip content.
+To manually test, copy the content in the first box and paste it into the second box.</p>
+<div id="source" contenteditable style="border: solid 1px; black; margin: 5px;">
+<font face="Verdana">hello&nbsp;</font><br>
+<font face="Verdana"><div style="-webkit-line-break: after-white-space;"><div><font face="Verdana">world</font></div>
+<div style="-webkit-line-break: after-white-space; "><font face="Verdana">WebKit</font></div></div>
+</div>
+<div id="destination" contenteditable style="border: solid 1px black; margin: 5px;"><br></div>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+
+Markup.description(document.getElementById('description').textContent);
+
+document.getElementById('source').focus();
+document.execCommand('selectall', false, null);
+document.execCommand('copy', false, null);
+if (document.queryCommandEnabled('paste')) {
+    document.getElementById('destination').focus();
+    document.execCommand('paste', false, null);
+
+    Markup.dump('source', 'Original content');
+    Markup.dump('destination', 'Pasted content');
+}
+
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2013-06-28  Ryosuke Niwa  <rniwa@webkit.org>
+
+        -webkit-line-break: after-white-space sometimes truncates DOM on copy & paste
+        https://bugs.webkit.org/show_bug.cgi?id=118164
+
+        Reviewed by Sam Weinig.
+
+        We can't assume that all subsequent ancestors contain exactly one child since they could have been
+        added in the first if statement matching: currentNode->parentNode() != rootNode && isRemovableBlock(currentNode)
+
+        Exit early when we encounter such an ancestor since removing its ancestor (that contains multiple children
+        some of which aren't in nodesToRemove) can clobber more nodes than we're allowed to remove.
+
+        Test: editing/pasteboard/simplfiying-markup-should-not-strip-content.html
+
+        * editing/SimplifyMarkupCommand.cpp:
+        (WebCore::SimplifyMarkupCommand::doApply):
+        (WebCore::SimplifyMarkupCommand::pruneSubsequentAncestorsToRemove):
+
 2013-06-28  Gwang Yoon Hwang  <ryumiel@company100.net>
 
         Coordinated Graphics: Separate CoordinatedLayerTreeHost into CoordinatedLayerTreeHost and CompositingCoordinator

diff --git a/Source/WebCore/editing/SimplifyMarkupCommand.cpp b/Source/WebCore/editing/SimplifyMarkupCommand.cpp
@@ -44,6 +44,8 @@ void SimplifyMarkupCommand::doApply()
     Node* rootNode = m_firstNode->parentNode();
     Vector<RefPtr<Node> > nodesToRemove;
 
+    document()->updateLayoutIgnorePendingStylesheets();
+
     // Walk through the inserted nodes, to see if there are elements that could be removed
     // without affecting the style. The goal is to produce leaner markup even when starting
     // from a verbose fragment.
@@ -102,7 +104,8 @@ int SimplifyMarkupCommand::pruneSubsequentAncestorsToRemove(Vector<RefPtr<Node>
     for (; pastLastNodeToRemove < nodesToRemove.size(); ++pastLastNodeToRemove) {
         if (nodesToRemove[pastLastNodeToRemove - 1]->parentNode() != nodesToRemove[pastLastNodeToRemove])
             break;
-        ASSERT(nodesToRemove[pastLastNodeToRemove]->firstChild() == nodesToRemove[pastLastNodeToRemove]->lastChild());
+        if (nodesToRemove[pastLastNodeToRemove]->firstChild() != nodesToRemove[pastLastNodeToRemove]->lastChild())
+            break;
     }
 
     Node* highestAncestorToRemove = nodesToRemove[pastLastNodeToRemove - 1].get();