Title: [220398] trunk
Revision
220398
Author
jfernan...@igalia.com
Date
2017-08-08 05:22:33 -0700 (Tue, 08 Aug 2017)

Log Message

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (220397 => 220398)


--- trunk/LayoutTests/ChangeLog	2017-08-08 10:27:11 UTC (rev 220397)
+++ trunk/LayoutTests/ChangeLog	2017-08-08 12:22:33 UTC (rev 220398)
@@ -1,3 +1,29 @@
+2017-08-08  Javier Fernandez  <jfernan...@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_hs...@apple.com>
 
         [iOS WK2] WKWebView schedules nonstop layout after pressing cmb+b,i,u inside a contenteditable div

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-1-expected.txt (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-1-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-1-expected.txt	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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"

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-1.html (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-1.html	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-1.html	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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=""
+    <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>

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-2-expected.txt (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-2-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-2-expected.txt	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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"

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-2.html (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-2.html	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-2.html	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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=""
+    <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>

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-3-expected.txt (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-3-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-3-expected.txt	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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>

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-3.html (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-3.html	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-3.html	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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=""
+    <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>

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-4-expected.txt (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-4-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-4-expected.txt	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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>

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-4.html (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-4.html	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-4.html	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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=""
+    <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>

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-5-expected.txt (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-5-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-5-expected.txt	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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"

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-5.html (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-5.html	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-5.html	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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=""
+    <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>

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-6-expected.txt (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-6-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-6-expected.txt	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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>

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-6.html (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-6.html	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-6.html	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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=""
+    <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>

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-7-expected.txt (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-7-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-7-expected.txt	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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"

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-7.html (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-7.html	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-7.html	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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=""
+    <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>

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-8-expected.txt (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-8-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-8-expected.txt	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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>

Added: trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-8.html (0 => 220398)


--- trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-8.html	                        (rev 0)
+++ trunk/LayoutTests/editing/deleting/delete-list-items-in-table-cell-8.html	2017-08-08 12:22:33 UTC (rev 220398)
@@ -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=""
+    <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>

Modified: trunk/Source/WebCore/ChangeLog (220397 => 220398)


--- trunk/Source/WebCore/ChangeLog	2017-08-08 10:27:11 UTC (rev 220397)
+++ trunk/Source/WebCore/ChangeLog	2017-08-08 12:22:33 UTC (rev 220398)
@@ -1,3 +1,26 @@
+2017-08-08  Javier Fernandez  <jfernan...@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  <zdober...@igalia.com>
 
         [TexMap] Isolate the TextureMapperPlatformLayerProxyProvider class

Modified: trunk/Source/WebCore/editing/TypingCommand.cpp (220397 => 220398)


--- trunk/Source/WebCore/editing/TypingCommand.cpp	2017-08-08 10:27:11 UTC (rev 220397)
+++ trunk/Source/WebCore/editing/TypingCommand.cpp	2017-08-08 12:22:33 UTC (rev 220398)
@@ -669,7 +669,11 @@
         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 @@
                 }
                 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;
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to