Not possible to remove the 'li' element inside the table cell

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: https://svn.webkit.org/repository/webkit/trunk@220398 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 16383c4447976646e828e07e5e8bbfe39520cb63..644af4074e06508529aed22acf000bded8991b49 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-1-expected.txt
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-1.html
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-2-expected.txt
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-2.html
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-3-expected.txt
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-3.html
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-4-expected.txt
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-4.html
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-5-expected.txt
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-5.html
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-6-expected.txt
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-6.html
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-7-expected.txt
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-7.html
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-8-expected.txt
@@ -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
+++ b/LayoutTests/editing/deleting/delete-list-items-in-table-cell-8.html
@@ -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>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 17bfdd858c6d3beae92423b322cd26f94ff0ddd8..72ec0b8176afa4e354f1526f4f40e1e5a9225cdf 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -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

diff --git a/Source/WebCore/editing/TypingCommand.cpp b/Source/WebCore/editing/TypingCommand.cpp
index 7bd95cc08b3526bdf373a4dfc4f7647b0095782e..9620bfb71270ccdc3635adfddafef321a608940f 100644 (file)
--- a/Source/WebCore/editing/TypingCommand.cpp
+++ b/Source/WebCore/editing/TypingCommand.cpp
@@ -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;