Merge r168460 - Dragging text from one paragraph to another does not …

…render as expected https://bugs.webkit.org/show_bug.cgi?id=132633 Reviewed by Darin Adler and Ryosuke Niwa. Source/WebCore: When we are dragging and dropping into a content editable field, we detect if we are trying to put a <p> into an existing <p>, and if so, split the outer <p> and insert the new <p> as its sibling. However, the outer <p> might not be editable, so we don't want to do any splitting and inserting at that location. Test: editing/pasteboard/drag-drop-paragraph-crasher.html * editing/ReplaceSelectionCommand.cpp: (WebCore::ReplaceSelectionCommand::makeInsertedContentRoundTrippableWithHTMLTreeBuilder): LayoutTests: The problem occurs when dragging text that includes a <p> into an editable area that has a <p> as a parent. * editing/pasteboard/drag-drop-paragraph-crasher-expected.txt: Added. * editing/pasteboard/drag-drop-paragraph-crasher.html: Added.
WebKit · May 22, 2014 · 9b889e7 · 9b889e7
1 parent 9048f58
commit 9b889e7
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 2 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2014-05-06  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Dragging text from one paragraph to another does not render as expected
+        https://bugs.webkit.org/show_bug.cgi?id=132633
+
+        Reviewed by Darin Adler and Ryosuke Niwa.
+
+        The problem occurs when dragging text that includes a <p> into an editable
+        area that has a <p> as a parent.
+
+        * editing/pasteboard/drag-drop-paragraph-crasher-expected.txt: Added.
+        * editing/pasteboard/drag-drop-paragraph-crasher.html: Added.
+
 2014-04-22  Brent Fulgham  <bfulgham@apple.com>
 
         Check (rather than assume) element is a RenderTableSection before using it

diff --git a/LayoutTests/editing/pasteboard/drag-drop-paragraph-crasher-expected.txt b/LayoutTests/editing/pasteboard/drag-drop-paragraph-crasher-expected.txt
@@ -0,0 +1,4 @@
+This tests text selection drag including a <p> tag, where its parent <p> tag is not editable.
+| "and drag it here<#selection-anchor>Select"
+| <p>
+|   "me<#selection-focus>"
diff --git a/LayoutTests/editing/pasteboard/drag-drop-paragraph-crasher.html b/LayoutTests/editing/pasteboard/drag-drop-paragraph-crasher.html
@@ -0,0 +1,35 @@
+<html>
+<body>
+<p>This tests text selection drag including a &lt;p&gt; tag, where its parent &lt;p&gt; tag is not editable.</p>
+To test this by hand, select the relevant text in this editable span:
+    <div id=source>Select
+        <p>me</p>
+    </div>
+    <p id=destination contenteditable>and drag it here</p>
+<br><br>If there is no crash, then the test passed.
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+function selectAllOfSource() {
+    var selection = window.getSelection();
+    var range = document.createRange();
+    range.selectNodeContents(document.getElementById("source"));
+    selection.removeAllRanges();
+    selection.addRange(range);
+}
+if (window.testRunner) {
+    Markup.description(document.querySelector('p').innerText);
+    selectAllOfSource();
+
+    eventSender.mouseMoveTo(source.offsetLeft + 10, source.offsetTop + source.offsetHeight / 2);
+    eventSender.mouseDown();
+    var destination = document.getElementById("destination");
+    eventSender.leapForward(500);
+    eventSender.mouseMoveTo(destination.offsetLeft + 700, destination.offsetTop + destination.offsetHeight / 2);
+    eventSender.mouseUp();
+
+    Markup.dump("destination");
+} else
+    Markup.noAutoDump();
+</script>
+</body>
+</html>
diff --git a/LayoutTests/platform/mac-wk2/TestExpectations b/LayoutTests/platform/mac-wk2/TestExpectations
@@ -113,6 +113,7 @@ media/controls-right-click-on-timebar.html
 platform/mac/editing/pasteboard/dataTransfer-set-data-file-url.html
 platform/mac/editing/pasteboard/drag-selections-to-contenteditable.html
 platform/mac/fast/forms/listbox-scrollbar-hit-test.html
+editing/pasteboard/drag-drop-paragraph-crasher.html
 
 # [WK2] [Mac] Spellcheck tests don't seem to work
 webkit.org/b/105616 editing/spelling/context-menu-suggestions-multiword-selection.html

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,21 @@
+2014-05-06  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Dragging text from one paragraph to another does not render as expected
+        https://bugs.webkit.org/show_bug.cgi?id=132633
+
+        Reviewed by Darin Adler and Ryosuke Niwa.
+
+        When we are dragging and dropping into a content editable field, we detect
+        if we are trying to put a <p> into an existing <p>, and if so, split the
+        outer <p> and insert the new <p> as its sibling. However, the outer <p>
+        might not be editable, so we don't want to do any splitting and inserting
+        at that location.
+
+        Test: editing/pasteboard/drag-drop-paragraph-crasher.html
+
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::makeInsertedContentRoundTrippableWithHTMLTreeBuilder):
+
 2014-05-07  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [SOUP] TLSErrors do not cause page load to fail when not ignored

diff --git a/Source/WebCore/editing/ReplaceSelectionCommand.cpp b/Source/WebCore/editing/ReplaceSelectionCommand.cpp
@@ -628,8 +628,11 @@ void ReplaceSelectionCommand::makeInsertedContentRoundTrippableWithHTMLTreeBuild
             continue;
 
         if (isProhibitedParagraphChild(toHTMLElement(node.get())->localName())) {
-            if (HTMLElement* paragraphElement = toHTMLElement(enclosingNodeWithTag(positionInParentBeforeNode(node.get()), pTag)))
-                moveNodeOutOfAncestor(node, paragraphElement);
+            if (auto* paragraphElement = toHTMLElement(enclosingNodeWithTag(positionInParentBeforeNode(node.get()), pTag))) {
+                auto* parent = paragraphElement->parentNode();
+                if (parent && parent->hasEditableStyle())
+                    moveNodeOutOfAncestor(node, paragraphElement);
+            }
         }
 
         if (isHeaderElement(node.get())) {