Attachment 192182 Details for Bug 110610 – Patch

[patch] Patch

bug-110610-20130308111047.patch (text/plain), 7.88 KB, created by Sergio Villar Senin on 2013-03-08 02:16:22 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Sergio Villar Senin

Created: 2013-03-08 02:16:22 PST

Size: 7.88 KB

patch

obsolete

>Subversion Revision: 144901
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 3f03a436870f2048eb4a66b299d4d36d4cfc7efb..27fc5fd13a521666933c812b478c8e031ebbc161 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2013-03-07 Sergio Villar Senin <svillar@igalia.com>
>+
>+ Empty list items after drag&drop in contentEditable divs
>+ https://bugs.webkit.org/show_bug.cgi?id=110610
>+
>+ Reviewed by NOBODY (OOPS!).
>+
>+ Perform a cleanup after moving operations. This will mainly prune
>+ extra placeholders left by the editing algorithms. Also do not
>+ leave empty <li> when moving them around inside a list element.
>+
>+ No new tests required, added the corresponding changes to the
>+ already existing tests.
>+
>+ * editing/MoveSelectionCommand.cpp:
>+ (WebCore::MoveSelectionCommand::doApply): perform
>+ cleanupAfterDeletion().
>+ * editing/ReplaceSelectionCommand.cpp:
>+ (WebCore::ReplaceSelectionCommand::insertAsListItems): do not
>+ insert an empty list item.
>+
> 2013-03-06 Sergio Villar Senin <svillar@igalia.com>
> 
> Improve drag&drop of list items in contentEditable divs
>diff --git a/Source/WebCore/editing/MoveSelectionCommand.cpp b/Source/WebCore/editing/MoveSelectionCommand.cpp
>index 5a2143c63d450555cef3980b79438f459dcfa134..475282ecd48c92815551ebbb47fa535e360884c3 100644
>--- a/Source/WebCore/editing/MoveSelectionCommand.cpp
>+++ b/Source/WebCore/editing/MoveSelectionCommand.cpp
>@@ -65,6 +65,8 @@ void MoveSelectionCommand::doApply()
> if (!pos.anchorNode()->inDocument())
> pos = endingSelection().start();
> 
>+ cleanupAfterDeletion(pos);
>+
> setEndingSelection(VisibleSelection(pos, endingSelection().affinity(), endingSelection().isDirectional()));
> if (!pos.anchorNode()->inDocument()) {
> // Document was modified out from under us.
>diff --git a/Source/WebCore/editing/ReplaceSelectionCommand.cpp b/Source/WebCore/editing/ReplaceSelectionCommand.cpp
>index 76d677aa06563c52db3d1d2a9d83df6534411bbe..d7a0b7336697307029e92e2fbab2be349c2a6584 100644
>--- a/Source/WebCore/editing/ReplaceSelectionCommand.cpp
>+++ b/Source/WebCore/editing/ReplaceSelectionCommand.cpp
>@@ -1341,8 +1341,6 @@ Node* ReplaceSelectionCommand::insertAsListItems(PassRefPtr<HTMLElement> prpList
> }
> if (isStart || isMiddle)
> lastNode = lastNode->previousSibling();
>- if (isMiddle)
>- insertNodeAfter(createListItemElement(document()), lastNode);
> return lastNode;
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 1b9aec37838efdd24d6a8f6c0779d54f2d2065b4..f4a08c717d7d1bdbadc9c49684110d2238fb02dc 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,20 @@
>+2013-03-07 Sergio Villar Senin <svillar@igalia.com>
>+
>+ Empty list items after drag&drop in contentEditable divs
>+ https://bugs.webkit.org/show_bug.cgi?id=110610
>+
>+ Reviewed by NOBODY (OOPS!).
>+
>+ Some placeholders should not be left after performing a cleanup in
>+ move operations, this includes the empty list items generated when
>+ moving around list items inside a list.
>+
>+ * editing/pasteboard/paste-list-004-expected.txt:
>+ * editing/pasteboard/paste-list-004.html: removed two empty <li>
>+ * editing/selection/4895428-1-expected.txt: removed a .
>+ * editing/selection/4895428-4-expected.txt: ditto.
>+ * editing/selection/drag-list-item-expected.txt: ditto.
>+
> 2013-03-06 Sergio Villar Senin <svillar@igalia.com>
> 
> Improve drag&drop of list items in contentEditable divs
>diff --git a/LayoutTests/editing/pasteboard/paste-list-004-expected.txt b/LayoutTests/editing/pasteboard/paste-list-004-expected.txt
>index 4c60579e1727eabd73d3a29f24d6d34bb0366981..c44b249d07304e9d99dbe43499a4cb681693ef21 100644
>--- a/LayoutTests/editing/pasteboard/paste-list-004-expected.txt
>+++ b/LayoutTests/editing/pasteboard/paste-list-004-expected.txt
>@@ -3,11 +3,9 @@ Pasting a list item into the middle of another list item should split the target
> one two: one two
> three: three
> one two: one two
>-:
> four: four
> monsters walking: monsters walking
> one two: one two
>-:
> cross the floor: cross the floor
> PASS
> 
>diff --git a/LayoutTests/editing/pasteboard/paste-list-004.html b/LayoutTests/editing/pasteboard/paste-list-004.html
>index 88d7c23ddc22d98e4e20893d49d8aff9ee35feda..8828e741f8f459be97572bfd313e0d2bcd1db194 100644
>--- a/LayoutTests/editing/pasteboard/paste-list-004.html
>+++ b/LayoutTests/editing/pasteboard/paste-list-004.html
>@@ -29,8 +29,7 @@ function editingTest()
> pasteCommand();
> 
> // Place the cursor between "walking" and "cross"
>- for (var i = 0; i < 2; ++i)
>- moveSelectionForwardByLineCommand();
>+ moveSelectionForwardByLineCommand();
> for (var i = 0; i < 2; ++i)
> moveSelectionForwardByWordCommand();
> 
>@@ -42,24 +41,22 @@ function editingTest()
> "one two",
> "three",
> "one two",
>- "",
> "four",
> "monsters walking",
> "one two",
>- "",
> "cross the floor"
> ];
>- if (listItems.length != 9)
>- throw "Expected 8 list items, found " + listItems.length;
>+ if (listItems.length != 7)
>+ throw "Expected 7 list items, found " + listItems.length;
> for (var i = 0; i < results.length; ++i) {
> var actual = listItems[i].innerText.replace(/^\s+/g, "");
> if (results[i] != actual)
> throw "Unexpected list item: " + i + "," + results[i] + "," + listItems[i].innerText;
> }
> 
>- // Verify that the cursor is in the right place (on the blank line).
>+ // Verify that the cursor is in the right place (at the beginning of 'cross the floor').
> var selection = window.getSelection();
>- if (selection.baseNode != listItems[7] || selection.baseOffset != 0 || !selection.isCollapsed)
>+ if (selection.baseNode != listItems[6].firstChild.firstChild.firstChild || selection.baseOffset != 0 || !selection.isCollapsed)
> throw "Wrong selection position";
> 
> for (var i = 0; i < listItems.length; ++i) {
>diff --git a/LayoutTests/editing/selection/4895428-1-expected.txt b/LayoutTests/editing/selection/4895428-1-expected.txt
>index e83b06b1718c2c6db825fe2efb19196362046cfb..1e932294d2828fe3d22c6ea1e079344ead24120e 100644
>--- a/LayoutTests/editing/selection/4895428-1-expected.txt
>+++ b/LayoutTests/editing/selection/4895428-1-expected.txt
>@@ -5,7 +5,6 @@ This tests dragging a selected table by mousing down on the contents of one of i
> | contenteditable="true"
> | id="source"
> | style="border: 1px solid blue;"
>-| 
> | "
> "
> | <div>
>diff --git a/LayoutTests/editing/selection/4895428-4-expected.txt b/LayoutTests/editing/selection/4895428-4-expected.txt
>index a2a53879113ed05039b125e449587b38c26f25e3..0eb9e4733808123afccda09b09558d6e66f17f1f 100644
>--- a/LayoutTests/editing/selection/4895428-4-expected.txt
>+++ b/LayoutTests/editing/selection/4895428-4-expected.txt
>@@ -7,7 +7,6 @@ This tests to see if an editable selection containing a focusable node is dragga
> | style="border: 1px solid blue;"
> | "
> "
>-| 
> | "
> "
> | "
>diff --git a/LayoutTests/editing/selection/drag-list-item-expected.txt b/LayoutTests/editing/selection/drag-list-item-expected.txt
>index f68482a87a81b2464701f119cccdf4a473b84da1..cb49b102624de9c461542b33c13209be987bacf5 100644
>--- a/LayoutTests/editing/selection/drag-list-item-expected.txt
>+++ b/LayoutTests/editing/selection/drag-list-item-expected.txt
>@@ -25,9 +25,6 @@ The original list looks like this. 'two' is going to be selected and pasted afte
> | id="one"
> | "one"
> | <li>
>-| id="two"
>-| 
>-| <li>
> | id="three"
> | "three"
> | <li>
>@@ -58,9 +55,6 @@ The original list looks like this. 'two' and 'three' are going to be selected an
> | id="one"
> | "one"
> | <li>
>-| id="two"
>-| 
>-| <li>
> | id="four"
> | "four"
> | <li>

Actions: View | Formatted Diff | Diff

Attachments on bug 110610: 190972 | 191725 | 192021 | 192036 | 192182 | 192431 | 192894 | 192897 | 193092