Not possible to remove the 'li' element inside the table cell
authorjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Aug 2017 12:22:33 +0000 (12:22 +0000)
committerjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Aug 2017 12:22:33 +0000 (12:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173148

Reviewed by Ryosuke Niwa.

Source/WebCore:

We need to add a new case for breaking out empty list items when they are
at the start of an editable area. Since list items can be also inside
table cells, we need to consider this kind of elements as well.

Tests: editing/deleting/delete-list-items-in-table-cell-1.html
       editing/deleting/delete-list-items-in-table-cell-2.html
       editing/deleting/delete-list-items-in-table-cell-3.html
       editing/deleting/delete-list-items-in-table-cell-4.html
       editing/deleting/delete-list-items-in-table-cell-5.html
       editing/deleting/delete-list-items-in-table-cell-6.html
       editing/deleting/delete-list-items-in-table-cell-7.html
       editing/deleting/delete-list-items-in-table-cell-8.html

* editing/TypingCommand.cpp:
(WebCore::TypingCommand::deleteKeyPressed):

LayoutTests:

Regression tests for different scenarios of list items removal.

* editing/deleting/delete-list-items-in-table-cell-1-expected.txt: Added.
* editing/deleting/delete-list-items-in-table-cell-1.html: Added.
* editing/deleting/delete-list-items-in-table-cell-2-expected.txt: Added.
* editing/deleting/delete-list-items-in-table-cell-2.html: Added.
* editing/deleting/delete-list-items-in-table-cell-3-expected.txt: Added.
* editing/deleting/delete-list-items-in-table-cell-3.html: Added.
* editing/deleting/delete-list-items-in-table-cell-4-expected.txt: Added.
* editing/deleting/delete-list-items-in-table-cell-4.html: Added.
* editing/deleting/delete-list-items-in-table-cell-5-expected.txt: Added.
* editing/deleting/delete-list-items-in-table-cell-5.html: Added.
* editing/deleting/delete-list-items-in-table-cell-6-expected.txt: Added.
* editing/deleting/delete-list-items-in-table-cell-6.html: Added.
* editing/deleting/delete-list-items-in-table-cell-7-expected.txt: Added.
* editing/deleting/delete-list-items-in-table-cell-7.html: Added.
* editing/deleting/delete-list-items-in-table-cell-8-expected.txt: Added.
* editing/deleting/delete-list-items-in-table-cell-8.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@220398 268f45cc-cd09-0410-ab3c-d52691b4dbfc

19 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/deleting/delete-list-items-in-table-cell-1-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-1.html [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-2-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-2.html [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-3-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-3.html [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-4-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-4.html [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-5-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-5.html [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-6-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-6.html [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-7-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-7.html [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-8-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-list-items-in-table-cell-8.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/TypingCommand.cpp

index 16383c4..644af40 100644 (file)
@@ -1,3 +1,29 @@
+2017-08-08  Javier Fernandez  <jfernandez@igalia.com>
+
+        Not possible to remove the 'li' element inside the table cell
+        https://bugs.webkit.org/show_bug.cgi?id=173148
+
+        Reviewed by Ryosuke Niwa.
+
+        Regression tests for different scenarios of list items removal.
+
+        * editing/deleting/delete-list-items-in-table-cell-1-expected.txt: Added.
+        * editing/deleting/delete-list-items-in-table-cell-1.html: Added.
+        * editing/deleting/delete-list-items-in-table-cell-2-expected.txt: Added.
+        * editing/deleting/delete-list-items-in-table-cell-2.html: Added.
+        * editing/deleting/delete-list-items-in-table-cell-3-expected.txt: Added.
+        * editing/deleting/delete-list-items-in-table-cell-3.html: Added.
+        * editing/deleting/delete-list-items-in-table-cell-4-expected.txt: Added.
+        * editing/deleting/delete-list-items-in-table-cell-4.html: Added.
+        * editing/deleting/delete-list-items-in-table-cell-5-expected.txt: Added.
+        * editing/deleting/delete-list-items-in-table-cell-5.html: Added.
+        * editing/deleting/delete-list-items-in-table-cell-6-expected.txt: Added.
+        * editing/deleting/delete-list-items-in-table-cell-6.html: Added.
+        * editing/deleting/delete-list-items-in-table-cell-7-expected.txt: Added.
+        * editing/deleting/delete-list-items-in-table-cell-7.html: Added.
+        * editing/deleting/delete-list-items-in-table-cell-8-expected.txt: Added.
+        * editing/deleting/delete-list-items-in-table-cell-8.html: Added.
+
 2017-08-08  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [iOS WK2] WKWebView schedules nonstop layout after pressing cmb+b,i,u inside a contenteditable div
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-1-expected.txt b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-1-expected.txt
new file mode 100644 (file)
index 0000000..e8429e8
--- /dev/null
@@ -0,0 +1,22 @@
+When deleting the second ordered list items in a table cell its content is merged with the first list item:
+
+Before:
+| <table>
+|   <tbody>
+|     <tr>
+|       <td>
+|         <ol>
+|           <li>
+|             "a"
+|           <li>
+|             id="li"
+|             "<#selection-caret>|b"
+
+After:
+| <table>
+|   <tbody>
+|     <tr>
+|       <td>
+|         <ol>
+|           <li>
+|             "a<#selection-caret>|b"
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-1.html b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-1.html
new file mode 100644 (file)
index 0000000..c80bd1a
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+    <div id="container" contenteditable="true"><table><tr><td><ol><li>a</li><li id="li">|b</li></ol></td></tr></table></div>
+    <script src="../../resources/dump-as-markup.js"></script>
+    <script>
+        Markup.description('When deleting the second ordered list items in a table cell its content is merged with the first list item:');
+
+        getSelection().collapse(document.getElementById("li"), 0);
+        Markup.dump('container', 'Before');
+
+        document.execCommand("Delete");
+        Markup.dump('container', 'After');
+    </script>
+</body>
+</html>
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-2-expected.txt b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-2-expected.txt
new file mode 100644 (file)
index 0000000..bab6710
--- /dev/null
@@ -0,0 +1,24 @@
+When deleting the second unordered list items in a table cell its content is merged with the first list item:
+
+Before:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         <ul>
+|           <li>
+|             "a"
+|           <li>
+|             id="li"
+|             "<#selection-caret>|b"
+
+After:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         <ul>
+|           <li>
+|             "a<#selection-caret>|b"
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-2.html b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-2.html
new file mode 100644 (file)
index 0000000..97be410
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+    <div id="container" contenteditable="true"><table border="1"><tr><td><ul><li>a</li><li id="li">|b</li></ul></td></tr></table></div>
+    <script src="../../resources/dump-as-markup.js"></script>
+    <script>
+        Markup.description('When deleting the second unordered list items in a table cell its content is merged with the first list item:');
+
+        getSelection().collapse(document.getElementById("li"), 0);
+        Markup.dump('container', 'Before');
+
+        document.execCommand("Delete");
+        Markup.dump('container', 'After');
+    </script>
+</body>
+</html>
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-3-expected.txt b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-3-expected.txt
new file mode 100644 (file)
index 0000000..50d5ec6
--- /dev/null
@@ -0,0 +1,23 @@
+When deleting the last ordered list item in a table cell we must delete the whole ordered list:
+
+Before:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         <ol>
+|           <li>
+|             id="li"
+|             <#selection-caret>
+|             <br>
+
+After:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         <div>
+|           <#selection-caret>
+|           <br>
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-3.html b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-3.html
new file mode 100644 (file)
index 0000000..a90b683
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+    <div id="container" contenteditable="true"><table border="1"><tr><td><ol><li id="li"><br></li></ol></td></tr></table></div>
+    <script src="../../resources/dump-as-markup.js"></script>
+    <script>
+        Markup.description('When deleting the last ordered list item in a table cell we must delete the whole ordered list:');
+
+        getSelection().collapse(document.getElementById("li"), 0);
+        Markup.dump('container', 'Before');
+
+        document.execCommand("Delete");
+        Markup.dump('container', 'After');
+    </script>
+</body>
+</html>
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-4-expected.txt b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-4-expected.txt
new file mode 100644 (file)
index 0000000..464e3a4
--- /dev/null
@@ -0,0 +1,23 @@
+When deleting the last unordered list item in a table cell we must delete the whole ordered list:
+
+Before:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         <ul>
+|           <li>
+|             id="li"
+|             <#selection-caret>
+|             <br>
+
+After:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         <div>
+|           <#selection-caret>
+|           <br>
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-4.html b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-4.html
new file mode 100644 (file)
index 0000000..1d3418d
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+    <div id="container" contenteditable="true"><table border="1"><tr><td><ul><li id="li"><br></li></ul></td></tr></table></div>
+    <script src="../../resources/dump-as-markup.js"></script>
+    <script>
+        Markup.description('When deleting the last unordered list item in a table cell we must delete the whole ordered list:');
+
+        getSelection().collapse(document.getElementById("li"), 0);
+        Markup.dump('container', 'Before');
+
+        document.execCommand("Delete");
+        Markup.dump('container', 'After');
+    </script>
+</body>
+</html>
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-5-expected.txt b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-5-expected.txt
new file mode 100644 (file)
index 0000000..67b4625
--- /dev/null
@@ -0,0 +1,26 @@
+When deleting the second ordered list items in a table cell, that it is the root editable element, its content is merged with the first list item:
+
+Before:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         contenteditable="true"
+|         <ol>
+|           <li>
+|             "a"
+|           <li>
+|             id="li"
+|             "<#selection-caret>|b"
+
+After:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         contenteditable="true"
+|         <ol>
+|           <li>
+|             "a<#selection-caret>|b"
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-5.html b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-5.html
new file mode 100644 (file)
index 0000000..8f60d42
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+    <div id="container"><table border="1"><tr><td contenteditable="true"><ol><li>a</li><li id="li">|b</li></ol></td></tr></table></div>
+    <script src="../../resources/dump-as-markup.js"></script>
+    <script>
+        Markup.description('When deleting the second ordered list items in a table cell, that it is the root editable element, its content is merged with the first list item:');
+
+        getSelection().collapse(document.getElementById("li"), 0);
+        Markup.dump('container', 'Before');
+
+        document.execCommand("Delete");
+        Markup.dump('container', 'After');
+    </script>
+</body>
+</html>
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-6-expected.txt b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-6-expected.txt
new file mode 100644 (file)
index 0000000..7a98df3
--- /dev/null
@@ -0,0 +1,25 @@
+When deleting the last ordered list item in a table cell, that it is the root editable element, we must delete the whole ordered list:
+
+Before:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         contenteditable="true"
+|         <ol>
+|           <li>
+|             id="li"
+|             <#selection-caret>
+|             <br>
+
+After:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         contenteditable="true"
+|         <div>
+|           <#selection-caret>
+|           <br>
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-6.html b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-6.html
new file mode 100644 (file)
index 0000000..6631bc5
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+    <div id="container"><table border="1"><tr><td contenteditable="true"><ol><li id="li"><br></li></ol></td></tr></table></div>
+    <script src="../../resources/dump-as-markup.js"></script>
+    <script>
+        Markup.description('When deleting the last ordered list item in a table cell, that it is the root editable element, we must delete the whole ordered list:');
+
+        getSelection().collapse(document.getElementById("li"), 0);
+        Markup.dump('container', 'Before');
+
+        document.execCommand("Delete");
+        Markup.dump('container', 'After');
+    </script>
+</body>
+</html>
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-7-expected.txt b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-7-expected.txt
new file mode 100644 (file)
index 0000000..636964f
--- /dev/null
@@ -0,0 +1,28 @@
+When deleting the second ordered list items in a table cell, which entire content is a contenteditable area, its content is merged with the first list item:
+
+Before:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         <div>
+|           contenteditable="true"
+|           <ol>
+|             <li>
+|               "a"
+|             <li>
+|               id="li"
+|               "<#selection-caret>|b"
+
+After:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         <div>
+|           contenteditable="true"
+|           <ol>
+|             <li>
+|               "a<#selection-caret>|b"
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-7.html b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-7.html
new file mode 100644 (file)
index 0000000..9411f8e
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+    <div id="container"><table border="1"><tr><td><div contenteditable="true"><ol><li>a</li><li id="li">|b</li></ol></div></td></tr></table></div>
+    <script src="../../resources/dump-as-markup.js"></script>
+    <script>
+        Markup.description('When deleting the second ordered list items in a table cell, which entire content is a contenteditable area, its content is merged with the first list item:');
+
+        getSelection().collapse(document.getElementById("li"), 0);
+        Markup.dump('container', 'Before');
+
+        document.execCommand("Delete");
+        Markup.dump('container', 'After');
+    </script>
+</body>
+</html>
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-8-expected.txt b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-8-expected.txt
new file mode 100644 (file)
index 0000000..45f3c42
--- /dev/null
@@ -0,0 +1,27 @@
+When deleting the last ordered list item in a table cell, which entire content is a contenteditable area, we must delete the whole ordered list:
+
+Before:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         <div>
+|           contenteditable="true"
+|           <ol>
+|             <li>
+|               id="li"
+|               <#selection-caret>
+|               <br>
+
+After:
+| <table>
+|   border="1"
+|   <tbody>
+|     <tr>
+|       <td>
+|         <div>
+|           contenteditable="true"
+|           <div>
+|             <#selection-caret>
+|             <br>
diff --git a/LayoutTests/editing/deleting/delete-list-items-in-table-cell-8.html b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-8.html
new file mode 100644 (file)
index 0000000..3df34ac
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+    <div id="container"><table border="1"><tr><td><div contenteditable="true"><ol><li id="li"><br></li></ol></div></td></tr></table></div>
+    <script src="../../resources/dump-as-markup.js"></script>
+    <script>
+        Markup.description('When deleting the last ordered list item in a table cell, which entire content is a contenteditable area, we must delete the whole ordered list:');
+
+        getSelection().collapse(document.getElementById("li"), 0);
+        Markup.dump('container', 'Before');
+
+        document.execCommand("Delete");
+        Markup.dump('container', 'After');
+    </script>
+</body>
+</html>
index 17bfdd8..72ec0b8 100644 (file)
@@ -1,3 +1,26 @@
+2017-08-08  Javier Fernandez  <jfernandez@igalia.com>
+
+        Not possible to remove the 'li' element inside the table cell
+        https://bugs.webkit.org/show_bug.cgi?id=173148
+
+        Reviewed by Ryosuke Niwa.
+
+        We need to add a new case for breaking out empty list items when they are
+        at the start of an editable area. Since list items can be also inside
+        table cells, we need to consider this kind of elements as well.
+
+        Tests: editing/deleting/delete-list-items-in-table-cell-1.html
+               editing/deleting/delete-list-items-in-table-cell-2.html
+               editing/deleting/delete-list-items-in-table-cell-3.html
+               editing/deleting/delete-list-items-in-table-cell-4.html
+               editing/deleting/delete-list-items-in-table-cell-5.html
+               editing/deleting/delete-list-items-in-table-cell-6.html
+               editing/deleting/delete-list-items-in-table-cell-7.html
+               editing/deleting/delete-list-items-in-table-cell-8.html
+
+        * editing/TypingCommand.cpp:
+        (WebCore::TypingCommand::deleteKeyPressed):
+
 2017-08-08  Zan Dobersek  <zdobersek@igalia.com>
 
         [TexMap] Isolate the TextureMapperPlatformLayerProxyProvider class
index 7bd95cc..9620bfb 100644 (file)
@@ -669,7 +669,11 @@ void TypingCommand::deleteKeyPressed(TextGranularity granularity, bool shouldAdd
         if (shouldAddToKillRing && selection.isCaret() && granularity != CharacterGranularity)
             selection.modify(FrameSelection::AlterationExtend, DirectionBackward, CharacterGranularity);
 
-        if (endingSelection().visibleStart().previous(CannotCrossEditingBoundary).isNull()) {
+        const VisiblePosition& visibleStart = endingSelection().visibleStart();
+        const VisiblePosition& previousPosition = visibleStart.previous(CannotCrossEditingBoundary);
+        Node* enclosingTableCell = enclosingNodeOfType(visibleStart.deepEquivalent(), &isTableCell);
+        const Node* enclosingTableCellForPreviousPosition = enclosingNodeOfType(previousPosition.deepEquivalent(), &isTableCell);
+        if (previousPosition.isNull() || enclosingTableCell != enclosingTableCellForPreviousPosition) {
             // When the caret is at the start of the editable area in an empty list item, break out of the list item.
             if (auto deleteListSelection = shouldBreakOutOfEmptyListItem()) {
                 if (willAddTypingToOpenCommand(DeleteKey, granularity, { }, Range::create(document(), deleteListSelection.value().start(), deleteListSelection.value().end()))) {
@@ -678,17 +682,17 @@ void TypingCommand::deleteKeyPressed(TextGranularity granularity, bool shouldAdd
                 }
                 return;
             }
+        }
+        if (previousPosition.isNull()) {
             // When there are no visible positions in the editing root, delete its entire contents.
             // FIXME: Dispatch a `beforeinput` event here and bail if preventDefault() was invoked.
-            if (endingSelection().visibleStart().next(CannotCrossEditingBoundary).isNull() && makeEditableRootEmpty()) {
+            if (visibleStart.next(CannotCrossEditingBoundary).isNull() && makeEditableRootEmpty()) {
                 typingAddedToOpenCommand(DeleteKey);
                 return;
             }
         }
 
-        VisiblePosition visibleStart(endingSelection().visibleStart());
         // If we have a caret selection at the beginning of a cell, we have nothing to do.
-        Node* enclosingTableCell = enclosingNodeOfType(visibleStart.deepEquivalent(), &isTableCell);
         if (enclosingTableCell && visibleStart == firstPositionInNode(enclosingTableCell))
             return;