Changeset 249307 in webkit

Timestamp:

Aug 29, 2019 6:40:58 PM (5 years ago)

Author:

timothy@apple.com

Message:

Copying and pasting two paragraphs with a newline between them results in stray paragraph with newline inside.
​https://bugs.webkit.org/show_bug.cgi?id=201306

Reviewed by Wenson Hsieh.

Source/WebCore:

Test: editing/pasteboard/paste-without-nesting.html

• editing/ReplaceSelectionCommand.cpp:

(WebCore::ReplaceSelectionCommand::moveNodeOutOfAncestor): Consider the ancestor node safe to remove
if there is no rendered text inside, not just if there are any child nodes.

LayoutTests:

• editing/pasteboard/paste-without-nesting-expected.txt: Updated results.
• editing/pasteboard/paste-without-nesting.html: Added new test case.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/pasteboard/paste-without-nesting-expected.txt (modified) (1 diff)
• LayoutTests/editing/pasteboard/paste-without-nesting.html (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/ReplaceSelectionCommand.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-08-29  Timothy Hatcher  <timothy@apple.com>
+
+        Copying and pasting two paragraphs with a newline between them results in stray paragraph with newline inside.
+        https://bugs.webkit.org/show_bug.cgi?id=201306
+
+        Reviewed by Wenson Hsieh.
+
+        * editing/pasteboard/paste-without-nesting-expected.txt: Updated results.
+        * editing/pasteboard/paste-without-nesting.html: Added new test case.
+
 2019-08-29  Devin Rousso  <drousso@apple.com>
 

trunk/LayoutTests/editing/pasteboard/paste-without-nesting-expected.txt

@@ -7 +7 @@
 PASS confirmedMarkup is '<div><b><i>Hello</i></b></div><div><b><i>world</i></b></div>'
 PASS confirmedMarkup is '<div style="text-align: center;"><div><font color="#ff0000"><b><i>hello</i></b></font></div><div><font color="#ff0000"><b><i>world</i></b></font></div></div>'
+PASS confirmedMarkup is '<p>Line One</p><p>Line Two</p>'
 PASS successfullyParsed is true
 

trunk/LayoutTests/editing/pasteboard/paste-without-nesting.html

@@ -41 +41 @@
 testCopyPaste("div", "<div><b><i>Hello</i></b></div><div><b><i>world</i></b></div>", "<div><b><i>Hello</i></b></div><div><b><i>world</i></b></div>");
 testCopyPaste("div", "<div style=\"text-align: center;\"><div><font color=\"#ff0000\"><b><i>hello</i></b></font></div><div><font color=\"#ff0000\"><b><i>world</i></b></font></div></div>", "<div style=\"text-align: center;\"><div><font color=\"#ff0000\"><b><i>hello</i></b></font></div><div><font color=\"#ff0000\"><b><i>world</i></b></font></div></div>");
+testCopyPaste("div", "<p>Line One</p>&#10;<p>Line Two</p>", "<p>Line One</p><p>Line Two</p>");
 
 root.style.display = "none";

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-08-29  Timothy Hatcher  <timothy@apple.com>
+
+        Copying and pasting two paragraphs with a newline between them results in stray paragraph with newline inside.
+        https://bugs.webkit.org/show_bug.cgi?id=201306
+
+        Reviewed by Wenson Hsieh.
+
+        Test: editing/pasteboard/paste-without-nesting.html
+
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::moveNodeOutOfAncestor): Consider the ancestor node safe to remove
+        if there is no rendered text inside, not just if there are any child nodes.
+
 2019-08-29  Devin Rousso  <drousso@apple.com>
 

trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp

@@ -705 +705 @@
 }
 
+static inline bool hasRenderedText(const Text& text)
+{
+    return text.renderer() && text.renderer()->hasRenderedText();
+}
+
 void ReplaceSelectionCommand::moveNodeOutOfAncestor(Node& node, Node& ancestor, InsertedNodes& insertedNodes)
 {
@@ -723 +728 @@
         insertNodeBefore(WTFMove(protectedNode), *nodeToSplitTo);
     }
-    if (!ancestor.firstChild()) {
+
+    document().updateLayoutIgnorePendingStylesheets();
+
+    bool safeToRemoveAncestor = true;
+    for (auto* child = ancestor.firstChild(); child; child = child->nextSibling()) {
+        if (is<Text>(child) && hasRenderedText(downcast<Text>(*child))) {
+            safeToRemoveAncestor = false;
+            break;
+        }
+
+        if (is<Element>(child)) {
+            safeToRemoveAncestor = false;
+            break;
+        }
+    }
+
+    if (safeToRemoveAncestor) {
         insertedNodes.willRemoveNode(&ancestor);
         removeNode(ancestor);
     }
-}
-
-static inline bool hasRenderedText(const Text& text)
-{
-    return text.renderer() && text.renderer()->hasRenderedText();
 }