Attachment 192026 Details for Bug 111556 – Patch

[patch] Patch

bug-111556-20130307184920.patch (text/plain), 8.72 KB, created by Sergio Villar Senin on 2013-03-07 09:54:54 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Sergio Villar Senin

Created: 2013-03-07 09:54:54 PST

Size: 8.72 KB

patch

obsolete

>Subversion Revision: 144901
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c518f65770925c1a946011c495b8ab39d3e84229..3f03a436870f2048eb4a66b299d4d36d4cfc7efb 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,23 @@
>+2013-03-06  Sergio Villar Senin  <svillar@igalia.com>
>+
>+        Improve drag&drop of list items in contentEditable divs
>+        https://bugs.webkit.org/show_bug.cgi?id=111556
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        A single fully selected <li> should generate the same markup as if
>+        we were selecting some of them, i.e., we should preserve the list
>+        structure and appearance.
>+
>+        Test: editing/selection/drag-list-item.html
>+
>+        * editing/htmlediting.cpp:
>+        (WebCore::isListItem): modified constness.
>+        * editing/htmlediting.h:
>+        (WebCore): ditto.
>+        * editing/markup.cpp:
>+        (WebCore::highestAncestorToWrapMarkup):
>+
> 2013-03-06  Allan Sandfeld Jensen  <allan.jensen@digia.com>
> 
>         REGGRESSION (r142112): It made compositing tests fail on EFL, Qt.
>diff --git a/Source/WebCore/editing/htmlediting.cpp b/Source/WebCore/editing/htmlediting.cpp
>index 24eb6eafe59f14f8c3598721c2331b5135f3acac..8a903247219415a939bbbe7bf698bac73043df2c 100644
>--- a/Source/WebCore/editing/htmlediting.cpp
>+++ b/Source/WebCore/editing/htmlediting.cpp
>@@ -580,7 +580,7 @@ bool isListElement(Node *n)
>     return (n && (n->hasTagName(ulTag) || n->hasTagName(olTag) || n->hasTagName(dlTag)));
> }
> 
>-bool isListItem(Node *n)
>+bool isListItem(const Node *n)
> {
>     return n && n->renderer() && n->renderer()->isListItem();
> }
>diff --git a/Source/WebCore/editing/htmlediting.h b/Source/WebCore/editing/htmlediting.h
>index fa63254768a1228f43fa1b64d0095bc2385f7bb5..5f67264d0b744c1acb58eed9e4cd17f66b767c5d 100644
>--- a/Source/WebCore/editing/htmlediting.h
>+++ b/Source/WebCore/editing/htmlediting.h
>@@ -107,7 +107,7 @@ bool isTableCell(const Node*);
> bool isEmptyTableCell(const Node*);
> bool isTableStructureNode(const Node*);
> bool isListElement(Node*);
>-bool isListItem(Node*);
>+bool isListItem(const Node*);
> bool isNodeRendered(const Node*);
> bool isNodeVisiblyContainedWithin(Node*, const Range*);
> bool isRenderedAsNonInlineTableImageOrHR(const Node*);
>diff --git a/Source/WebCore/editing/markup.cpp b/Source/WebCore/editing/markup.cpp
>index d3277c3666667a3da6292a7a79a517ee15412c5c..afa8388f5e3dd428de8933af777faacf87ba1086 100644
>--- a/Source/WebCore/editing/markup.cpp
>+++ b/Source/WebCore/editing/markup.cpp
>@@ -515,6 +515,14 @@ static Node* highestAncestorToWrapMarkup(const Range* range, EAnnotateForInterch
>         // the structure and appearance of the copied markup.
>         specialCommonAncestor = ancestorToRetainStructureAndAppearance(commonAncestor);
> 
>+        if (Node* parentListNode = enclosingNodeOfType(firstPositionInOrBeforeNode(range->firstNode()), isListItem)) {
>+            if (WebCore::areRangesEqual(VisibleSelection::selectionFromContentsOfNode(parentListNode).toNormalizedRange().get(), range)) {
>+                specialCommonAncestor = parentListNode->parentNode();
>+                while (specialCommonAncestor && !isListElement(specialCommonAncestor))
>+                    specialCommonAncestor = specialCommonAncestor->parentNode();
>+            }
>+        }
>+
>         // Retain the Mail quote level by including all ancestor mail block quotes.
>         if (Node* highestMailBlockquote = highestEnclosingNodeOfType(firstPositionInOrBeforeNode(range->firstNode()), isMailBlockquote, CanCrossEditingBoundary))
>             specialCommonAncestor = highestMailBlockquote;
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 058d4b6b4bd3986cfbe6dc22ed345f6bbfd5196e..1b9aec37838efdd24d6a8f6c0779d54f2d2065b4 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2013-03-06  Sergio Villar Senin  <svillar@igalia.com>
>+
>+        Improve drag&drop of list items in contentEditable divs
>+        https://bugs.webkit.org/show_bug.cgi?id=111556
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added a new test case. The test performs 2 drag&drops, first of
>+        all a selection of a single list item, and secondly a selection of
>+        two list items. It checks that both behave the same way, i.e, the
>+        whole <li> is moved and not only the text.
>+
>+        * editing/selection/drag-list-item-expected.txt: Added.
>+        * editing/selection/drag-list-item.html: Added.
>+
> 2013-03-06  Allan Sandfeld Jensen  <allan.jensen@digia.com>
> 
>         REGGRESSION (r142112): It made compositing tests fail on EFL, Qt.
>diff --git a/LayoutTests/editing/selection/drag-list-item-expected.txt b/LayoutTests/editing/selection/drag-list-item-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..f68482a87a81b2464701f119cccdf4a473b84da1
>--- /dev/null
>+++ b/LayoutTests/editing/selection/drag-list-item-expected.txt
>@@ -0,0 +1,71 @@
>+This test drags a selection of one <li> and drops them after another <li> in the first list. The same test is repeated in the second list but this time with two selected items instead of one. It verifies that the behavior is independent of how many fully selected items we move, i.e. fully selected <li> are moved as <li> and not just as plain text
>+
>+The original list looks like this. 'two' is going to be selected and pasted after 'four':
>+| "
>+"
>+| <li>
>+|   id="one"
>+|   "one"
>+| <li>
>+|   id="two"
>+|   "two"
>+| <li>
>+|   id="three"
>+|   "three"
>+| <li>
>+|   id="four"
>+|   "four"
>+| "
>+"
>+
>+'two' should appear as a new <li> after 'four':
>+| "
>+"
>+| <li>
>+|   id="one"
>+|   "one"
>+| <li>
>+|   id="two"
>+|   <br>
>+| <li>
>+|   id="three"
>+|   "three"
>+| <li>
>+|   id="four"
>+|   "four"
>+| <li>
>+|   id="two"
>+|   "two"
>+| "
>+"
>+
>+The original list looks like this. 'two' and 'three' are going to be selected and pasted after 'four':
>+| <li>
>+|   id="one"
>+|   "one"
>+| <li>
>+|   id="two"
>+|   "two"
>+| <li>
>+|   id="three"
>+|   "three"
>+| <li>
>+|   id="four"
>+|   "four"
>+
>+'two' and 'three' should appear as <li> after 'four':
>+| <li>
>+|   id="one"
>+|   "one"
>+| <li>
>+|   id="two"
>+|   <br>
>+| <li>
>+|   id="four"
>+|   "four"
>+| <li>
>+|   id="two"
>+|   "two"
>+| <li>
>+|   id="three"
>+|   "three"
>diff --git a/LayoutTests/editing/selection/drag-list-item.html b/LayoutTests/editing/selection/drag-list-item.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..97dd548290f3bfaaf8a5b87c532655bac62cdd9e
>--- /dev/null
>+++ b/LayoutTests/editing/selection/drag-list-item.html
>@@ -0,0 +1,54 @@
>+<!DOCTYPE html>
>+
>+<body>
>+<script src="../../resources/dump-as-markup.js"></script>
>+<div contenteditable="true">
>+<ol id="test">
>+<li id="one">one</li><li id="two">two</li><li id="three">three</li><li id="four">four</li>
>+</ol>
>+</div>
>+
>+<script>
>+Markup.description('This test drags a selection of one <li> and drops them after another <li> in the first list. The same test is repeated in the second list but this time with two selected items instead of one. It verifies that the behavior is independent of how many fully selected items we move, i.e. fully selected <li> are moved as <li> and not just as plain text');
>+
>+function editingTest(testElement, startSelectionElement, endSelectionElement, endSelectionElementOffset, targetElement) {
>+
>+    var testElement = document.getElementById(testElement);
>+    var selection = window.getSelection();
>+    selection.setPosition(testElement, 0);
>+
>+    var start = document.getElementById(startSelectionElement);
>+    var startx = start.offsetLeft;
>+    var starty = start.offsetTop + start.offsetHeight / 2;
>+    selection.setBaseAndExtent(start, 0, document.getElementById(endSelectionElement), endSelectionElementOffset);
>+
>+    eventSender.mouseMoveTo(startx, starty);
>+    eventSender.mouseDown();
>+
>+    eventSender.leapForward(200);
>+
>+    var target = document.getElementById(targetElement);
>+    var targetx = target.parentNode.offsetLeft + target.offsetLeft + target.offsetWidth / 2;
>+    var targety = target.offsetTop + target.offsetHeight / 2;
>+
>+    eventSender.mouseMoveTo(targetx, targety);
>+    eventSender.mouseUp();
>+
>+    selection.collapse();
>+}
>+
>+if (window.testRunner) {
>+
>+    Markup.dump("test", "The original list looks like this. 'two' is going to be selected and pasted after 'four'");
>+    editingTest("test", "two", "two", 4, "four");
>+    Markup.dump("test", "'two' should appear as a new <li> after 'four'");
>+
>+    document.getElementById("test").innerHTML = "<li id=\"one\">one</li><li id=\"two\">two</li>"
>+        + "<li id=\"three\">three</li><li id=\"four\">four</li>";
>+    Markup.dump("test", "The original list looks like this. 'two' and 'three' are going to be selected and pasted after 'four'");
>+    editingTest("test", "two", "three", 5, "four");
>+    Markup.dump("test", "'two' and 'three' should appear as <li> after 'four'");
>+}
>+</script>
>+</body>
>+</html>

Flags:

rniwa: review+
buildbot: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 111556: 191720 | 191807 | 192026