- Revision
- 233885
- Author
- jfernan...@igalia.com
- Date
- 2018-07-17 01:48:49 -0700 (Tue, 17 Jul 2018)
Log Message
Delete content of a single cell table should not delete the whole table
https://bugs.webkit.org/show_bug.cgi?id=173117
Reviewed by Ryosuke Niwa.
Source/WebCore:
We should not extend selection looking for special elements if the
delete operation has been triggered by a caret based selection.
This change is based on a recent [1] resolution of the Editing TF,
which acknowledges that behavior of single-cell tables must be the
same that multi-cell tables and even if the last character is
deleted, we should not delete the whole table structure.
A different case would be when the user actively selects the whole
content of a table; in this case, as we do in multi-cell tables,
the structure of single-cell tables should be deleted together
with the content.
[1] https://github.com/w3c/editing/issues/163
Tests: editing/deleting/backspace-delete-last-char-in-table.html
editing/deleting/forward-delete-last-char-in-table.html
editing/deleting/select-and-delete-last-char-in-table.html
* editing/TypingCommand.cpp:
(WebCore::TypingCommand::deleteKeyPressed):
(WebCore::TypingCommand::forwardDeleteKeyPressed):
LayoutTests:
Tests to verify that single-cell tables are not deleted when their
last character is deleted, unless it was previously selected by
the user.
Changes two expected files to adapt them to the new logic.
* LayoutTests/editing/deleting/deleting-relative-positioned-special-element-expected.txt: The paragraph is not deleted, even if it's empty. The paragraphs above are not merged, which was the goal of the test.
* editing/deleting/delete-last-char-in-table-expected.txt: The table is not removed, even if it's empty. The formatted elements are deleted, which was the goal of the test.
* editing/deleting/backspace-delete-last-char-in-table-expected.txt: Added.
* editing/deleting/backspace-delete-last-char-in-table.html: Added.
* editing/deleting/forward-delete-last-char-in-table-expected.txt: Added.
* editing/deleting/forward-delete-last-char-in-table.html: Added.
* editing/deleting/select-and-delete-last-char-in-table-expected.txt: Added.
* editing/deleting/select-and-delete-last-char-in-table.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (233884 => 233885)
--- trunk/LayoutTests/ChangeLog 2018-07-17 05:07:34 UTC (rev 233884)
+++ trunk/LayoutTests/ChangeLog 2018-07-17 08:48:49 UTC (rev 233885)
@@ -1,3 +1,25 @@
+2018-07-17 Javier Fernandez <jfernan...@igalia.com>
+
+ Delete content of a single cell table should not delete the whole table
+ https://bugs.webkit.org/show_bug.cgi?id=173117
+
+ Reviewed by Ryosuke Niwa.
+
+ Tests to verify that single-cell tables are not deleted when their
+ last character is deleted, unless it was previously selected by
+ the user.
+
+ Changes two expected files to adapt them to the new logic.
+
+ * LayoutTests/editing/deleting/deleting-relative-positioned-special-element-expected.txt: The paragraph is not deleted, even if it's empty. The paragraphs above are not merged, which was the goal of the test.
+ * editing/deleting/delete-last-char-in-table-expected.txt: The table is not removed, even if it's empty. The formatted elements are deleted, which was the goal of the test.
+ * editing/deleting/backspace-delete-last-char-in-table-expected.txt: Added.
+ * editing/deleting/backspace-delete-last-char-in-table.html: Added.
+ * editing/deleting/forward-delete-last-char-in-table-expected.txt: Added.
+ * editing/deleting/forward-delete-last-char-in-table.html: Added.
+ * editing/deleting/select-and-delete-last-char-in-table-expected.txt: Added.
+ * editing/deleting/select-and-delete-last-char-in-table.html: Added.
+
2018-07-16 Simon Fraser <simon.fra...@apple.com>
Roll out r233873 and r233875 since they caused 8 new layout test crashes.
Added: trunk/LayoutTests/editing/deleting/backspace-delete-last-char-in-table-expected.txt (0 => 233885)
--- trunk/LayoutTests/editing/deleting/backspace-delete-last-char-in-table-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/deleting/backspace-delete-last-char-in-table-expected.txt 2018-07-17 08:48:49 UTC (rev 233885)
@@ -0,0 +1,24 @@
+In this test the last character of a single-cell table is deleted, just using the backspace key. Only the table cell's content should be deleted, and not the table itself.
+
+BeforeDeletion:
+| "First"
+| <table>
+| border="1"
+| <tbody>
+| <tr>
+| <td>
+| id="test"
+| "1<#selection-caret>"
+| "Second"
+
+AfterDeletion:
+| "First"
+| <table>
+| border="1"
+| <tbody>
+| <tr>
+| <td>
+| id="test"
+| <#selection-caret>
+| <br>
+| "Second"
Added: trunk/LayoutTests/editing/deleting/backspace-delete-last-char-in-table.html (0 => 233885)
--- trunk/LayoutTests/editing/deleting/backspace-delete-last-char-in-table.html (rev 0)
+++ trunk/LayoutTests/editing/deleting/backspace-delete-last-char-in-table.html 2018-07-17 08:48:49 UTC (rev 233885)
@@ -0,0 +1,14 @@
+<!doctype html>
+<script src="" ></script>
+<script src=""
+<div contenteditable id="root">First<table border="1"><tr><td id="test">1</td></tr></table>Second</div>
+<script>
+Markup.description("In this test the last character of a single-cell table is deleted, just using the backspace key. Only the table cell's content should be deleted, and not the table itself.")
+
+const element = document.getElementById("test");
+getSelection().collapse(element, 1);
+Markup.dump('root', 'BeforeDeletion');
+
+deleteCommand();
+Markup.dump('root', 'AfterDeletion');
+</script>
Modified: trunk/LayoutTests/editing/deleting/delete-last-char-in-table-expected.txt (233884 => 233885)
--- trunk/LayoutTests/editing/deleting/delete-last-char-in-table-expected.txt 2018-07-17 05:07:34 UTC (rev 233884)
+++ trunk/LayoutTests/editing/deleting/delete-last-char-in-table-expected.txt 2018-07-17 08:48:49 UTC (rev 233885)
@@ -2,4 +2,4 @@
PASS
-execDeleteCommand: <br>
+execDeleteCommand: <table style="border-collapse:collapse"><tbody><tr><td id="cursor"><br></td></tr></tbody></table>
Modified: trunk/LayoutTests/editing/deleting/deleting-relative-positioned-special-element-expected.txt (233884 => 233885)
--- trunk/LayoutTests/editing/deleting/deleting-relative-positioned-special-element-expected.txt 2018-07-17 05:07:34 UTC (rev 233884)
+++ trunk/LayoutTests/editing/deleting/deleting-relative-positioned-special-element-expected.txt 2018-07-17 08:48:49 UTC (rev 233885)
@@ -14,9 +14,11 @@
AfterDeletion:
| <p>
| "1"
-| <#selection-caret>
-| <br>
| <p>
+| id="paragraphToDelete"
+| <#selection-caret>
+| <br>
+| <p>
| "3"
| <p>
| "4"
Added: trunk/LayoutTests/editing/deleting/forward-delete-last-char-in-table-expected.txt (0 => 233885)
--- trunk/LayoutTests/editing/deleting/forward-delete-last-char-in-table-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/deleting/forward-delete-last-char-in-table-expected.txt 2018-07-17 08:48:49 UTC (rev 233885)
@@ -0,0 +1,23 @@
+In this test the last character of a single-cell table is deleted, just using the forwardDelete key. Only the table cell's content should be deleted, and not the table itself.
+
+BeforeDeletion:
+| "First"
+| <table>
+| border="1"
+| <tbody>
+| <tr>
+| <td>
+| id="test"
+| "1<#selection-caret>"
+| "Second"
+
+AfterDeletion:
+| "First"
+| <table>
+| border="1"
+| <tbody>
+| <tr>
+| <td>
+| id="test"
+| "1<#selection-caret>"
+| "Second"
Added: trunk/LayoutTests/editing/deleting/forward-delete-last-char-in-table.html (0 => 233885)
--- trunk/LayoutTests/editing/deleting/forward-delete-last-char-in-table.html (rev 0)
+++ trunk/LayoutTests/editing/deleting/forward-delete-last-char-in-table.html 2018-07-17 08:48:49 UTC (rev 233885)
@@ -0,0 +1,14 @@
+<!doctype html>
+<script src="" ></script>
+<script src=""
+<div contenteditable id="root">First<table border="1"><tr><td id="test">1</td></tr></table>Second</div>
+<script>
+Markup.description("In this test the last character of a single-cell table is deleted, just using the forwardDelete key. Only the table cell's content should be deleted, and not the table itself.")
+
+const element = document.getElementById("test");
+getSelection().collapse(element, 1);
+Markup.dump('root', 'BeforeDeletion');
+
+forwardDeleteCommand();
+Markup.dump('root', 'AfterDeletion');
+</script>
Added: trunk/LayoutTests/editing/deleting/select-and-delete-last-char-in-table-expected.txt (0 => 233885)
--- trunk/LayoutTests/editing/deleting/select-and-delete-last-char-in-table-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/deleting/select-and-delete-last-char-in-table-expected.txt 2018-07-17 08:48:49 UTC (rev 233885)
@@ -0,0 +1,17 @@
+In this test the last character of a single-cell table is selected and then deleted using the backspace key. The whole table is deleted.
+
+BeforeDeletion:
+| "First"
+| <table>
+| border="1"
+| <tbody>
+| <tr>
+| <td>
+| id="test"
+| "<#selection-anchor>1<#selection-focus>"
+| "Second"
+
+AfterDeletion:
+| "First<#selection-caret>"
+| <br>
+| "Second"
Added: trunk/LayoutTests/editing/deleting/select-and-delete-last-char-in-table.html (0 => 233885)
--- trunk/LayoutTests/editing/deleting/select-and-delete-last-char-in-table.html (rev 0)
+++ trunk/LayoutTests/editing/deleting/select-and-delete-last-char-in-table.html 2018-07-17 08:48:49 UTC (rev 233885)
@@ -0,0 +1,15 @@
+<!doctype html>
+<script src="" ></script>
+<script src=""
+<div contenteditable id="root">First<table border="1"><tr><td id="test">1</td></tr></table>Second</div>
+<script>
+Markup.description("In this test the last character of a single-cell table is selected and then deleted using the backspace key. The whole table is deleted.")
+
+const element = document.getElementById("test");
+getSelection().collapse(element, 0);
+extendSelectionForwardByCharacterCommand();
+Markup.dump('root', 'BeforeDeletion');
+
+deleteCommand();
+Markup.dump('root', 'AfterDeletion');
+</script>
Modified: trunk/Source/WebCore/ChangeLog (233884 => 233885)
--- trunk/Source/WebCore/ChangeLog 2018-07-17 05:07:34 UTC (rev 233884)
+++ trunk/Source/WebCore/ChangeLog 2018-07-17 08:48:49 UTC (rev 233885)
@@ -1,3 +1,33 @@
+2018-07-17 Javier Fernandez <jfernan...@igalia.com>
+
+ Delete content of a single cell table should not delete the whole table
+ https://bugs.webkit.org/show_bug.cgi?id=173117
+
+ Reviewed by Ryosuke Niwa.
+
+ We should not extend selection looking for special elements if the
+ delete operation has been triggered by a caret based selection.
+
+ This change is based on a recent [1] resolution of the Editing TF,
+ which acknowledges that behavior of single-cell tables must be the
+ same that multi-cell tables and even if the last character is
+ deleted, we should not delete the whole table structure.
+
+ A different case would be when the user actively selects the whole
+ content of a table; in this case, as we do in multi-cell tables,
+ the structure of single-cell tables should be deleted together
+ with the content.
+
+ [1] https://github.com/w3c/editing/issues/163
+
+ Tests: editing/deleting/backspace-delete-last-char-in-table.html
+ editing/deleting/forward-delete-last-char-in-table.html
+ editing/deleting/select-and-delete-last-char-in-table.html
+
+ * editing/TypingCommand.cpp:
+ (WebCore::TypingCommand::deleteKeyPressed):
+ (WebCore::TypingCommand::forwardDeleteKeyPressed):
+
2018-07-16 Megan Gardner <megan_gard...@apple.com>
Correctly adjust scroll offsets when a page is zoomed
Modified: trunk/Source/WebCore/editing/TypingCommand.cpp (233884 => 233885)
--- trunk/Source/WebCore/editing/TypingCommand.cpp 2018-07-17 05:07:34 UTC (rev 233884)
+++ trunk/Source/WebCore/editing/TypingCommand.cpp 2018-07-17 08:48:49 UTC (rev 233885)
@@ -649,6 +649,7 @@
VisibleSelection selectionToDelete;
VisibleSelection selectionAfterUndo;
+ bool expandForSpecialElements = !endingSelection().isCaret();
switch (endingSelection().selectionType()) {
case VisibleSelection::RangeSelection:
@@ -760,7 +761,7 @@
// more text than you insert. In that case all of the text that was around originally should be selected.
if (m_openedByBackwardDelete)
setStartingSelection(selectionAfterUndo);
- CompositeEditCommand::deleteSelection(selectionToDelete, m_smartDelete);
+ CompositeEditCommand::deleteSelection(selectionToDelete, m_smartDelete, /* mergeBlocksAfterDelete*/ true, /* replace*/ false, expandForSpecialElements, /*sanitizeMarkup*/ true);
setSmartDelete(false);
typingAddedToOpenCommand(DeleteKey);
}
@@ -774,6 +775,7 @@
VisibleSelection selectionToDelete;
VisibleSelection selectionAfterUndo;
+ bool expandForSpecialElements = !endingSelection().isCaret();
switch (endingSelection().selectionType()) {
case VisibleSelection::RangeSelection:
@@ -862,7 +864,7 @@
frame.editor().addRangeToKillRing(*selectionToDelete.toNormalizedRange().get(), Editor::KillRingInsertionMode::AppendText);
// make undo select what was deleted
setStartingSelection(selectionAfterUndo);
- CompositeEditCommand::deleteSelection(selectionToDelete, m_smartDelete);
+ CompositeEditCommand::deleteSelection(selectionToDelete, m_smartDelete, /* mergeBlocksAfterDelete*/ true, /* replace*/ false, expandForSpecialElements, /*sanitizeMarkup*/ true);
setSmartDelete(false);
typingAddedToOpenCommand(ForwardDeleteKey);
}
