Commit 39714081 authored by Javier Fernandez's avatar Javier Fernandez Committed by Commit Bot

Don't extend selection for special elements on caret based deletes

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

Bug: 731320
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I8c3e4efcc63410bf30b4889a715d92e9dda8189b
Reviewed-on: https://chromium-review.googlesource.com/1126255
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575591}
parent 602d2720
......@@ -2,4 +2,4 @@ See bug 57148. When deleteing the last character in a table deletes the table, n
PASS
execDeleteCommand: <br>
execDeleteCommand: <table style="border-collapse:collapse"><tbody><tr><td id="cursor"><br></td></tr></tbody></table>
<!doctype html>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="../assert_selection.js"></script>
<script>
selection_test(
[
'<div contenteditable>',
'<div>',
'before',
'<table><tbody>',
'<tr><td>1|</td></tr>',
'</tbody></table>',
'after',
'</div>',
'</div>',
],
'delete',
[
'<div contenteditable>',
'before',
'<table><tbody>',
'<tr><td>|<br></td></tr>',
'</tbody></table>',
'after',
'</div>',
],
'1. Delete the last character in a single-cell table.');
selection_test(
[
'<div contenteditable>',
'<div>',
'before',
'<table><tbody>',
'<tr><td>|1</td></tr>',
'</tbody></table>',
'after',
'</div>',
'</div>',
],
'ForwardDelete',
[
'<div contenteditable>',
'before',
'<table><tbody>',
'<tr><td>|<br></td></tr>',
'</tbody></table>',
'after',
'</div>',
],
'2. forward-delete the last character in a single-cell table.');
selection_test(
[
'<div contenteditable>',
'<div>',
'before',
'<table><tbody>',
'<tr><td>|1^</td></tr>',
'</tbody></table>',
'after',
'</div>',
'</div>',
],
'delete',
'<div contenteditable>before<br>|after</div>',
'3. Select and delete last character in a single-cell table.');
selection_test(
[
'<div contenteditable>',
'<div>',
'before',
'<table><tbody>',
'<tr><td>1|</td><td></td></tr>',
'</tbody></table>',
'after',
'</div>',
'</div>',
],
'delete',
[
'<div contenteditable>',
'before',
'<table><tbody>',
'<tr><td>|<br></td><td></td></tr>',
'</tbody></table>',
'after',
'</div>',
],
'4. Delete the last character in a multiple-cell table.');
selection_test(
[
'<div contenteditable>',
'<div>',
'before',
'<table><tbody>',
'<tr><td>|1^</td><td></td></tr>',
'</tbody></table>',
'after',
'</div>',
'</div>',
],
'delete',
[
'<div contenteditable>',
'before',
'<table><tbody>',
'<tr><td>|<br></td><td></td></tr>',
'</tbody></table>',
'after',
'</div>',
],
'5. Select and delete the last character in a multiple-cell table.');
</script>
......@@ -520,6 +520,11 @@ var browserTests = [
"<div style=\"white-space:nowrap\">foo[]bar</div>",
[true],
{"delete":[false,false,"",false,false,""]}],
["foo<table><tr><td>b[]</table>baz",
[["delete",""]],
"foo<table><tbody><tr><td>[]<br></td></tr></tbody></table>baz",
[true],
{"delete":[false,false,"",false,false,""]}],
["foo<table><tr><td>[]bar</table>baz",
[["delete",""]],
"foo<table><tbody><tr><td>[]bar</td></tr></tbody></table>baz",
......
......@@ -215,6 +215,11 @@ var browserTests = [
"foo{}",
[true,true],
{"defaultparagraphseparator":[false,false,"div",false,false,"p"],"forwarddelete":[false,false,"",false,false,""]}],
["<table><tr><td>[]b</table>foo",
[["forwarddelete",""]],
"<table><tbody><tr><td>[]<br></td></tr></tbody></table>foo",
[true],
{"forwarddelete":[false,false,"",false,false,""]}],
["<table><tr><td>{}</table>foo",
[["forwarddelete",""]],
"<table><tbody><tr><td>{}</td></tr></tbody></table>foo",
......
This is a testharness.js-based test.
Found 6742 tests; 6568 PASS, 174 FAIL, 0 TIMEOUT, 0 NOTRUN.
Found 6751 tests; 6577 PASS, 174 FAIL, 0 TIMEOUT, 0 NOTRUN.
PASS [["delete",""]] "foo[]bar": execCommand("delete", false, "") return value
PASS [["delete",""]] "foo[]bar" checks for modifications to non-editable content
PASS [["delete",""]] "foo[]bar" compare innerHTML
......@@ -1118,6 +1118,15 @@ PASS [["delete",""]] "<div style=white-space:nowrap>foo []bar</div>" queryComma
PASS [["delete",""]] "<div style=white-space:nowrap>foo []bar</div>" queryCommandIndeterm("delete") after
PASS [["delete",""]] "<div style=white-space:nowrap>foo []bar</div>" queryCommandState("delete") after
PASS [["delete",""]] "<div style=white-space:nowrap>foo []bar</div>" queryCommandValue("delete") after
PASS [["delete",""]] "foo<table><tr><td>b[]</table>baz": execCommand("delete", false, "") return value
PASS [["delete",""]] "foo<table><tr><td>b[]</table>baz" checks for modifications to non-editable content
PASS [["delete",""]] "foo<table><tr><td>b[]</table>baz" compare innerHTML
PASS [["delete",""]] "foo<table><tr><td>b[]</table>baz" queryCommandIndeterm("delete") before
PASS [["delete",""]] "foo<table><tr><td>b[]</table>baz" queryCommandState("delete") before
PASS [["delete",""]] "foo<table><tr><td>b[]</table>baz" queryCommandValue("delete") before
PASS [["delete",""]] "foo<table><tr><td>b[]</table>baz" queryCommandIndeterm("delete") after
PASS [["delete",""]] "foo<table><tr><td>b[]</table>baz" queryCommandState("delete") after
PASS [["delete",""]] "foo<table><tr><td>b[]</table>baz" queryCommandValue("delete") after
PASS [["delete",""]] "foo<table><tr><td>[]bar</table>baz": execCommand("delete", false, "") return value
PASS [["delete",""]] "foo<table><tr><td>[]bar</table>baz" checks for modifications to non-editable content
PASS [["delete",""]] "foo<table><tr><td>[]bar</table>baz" compare innerHTML
......
This is a testharness.js-based test.
Found 6409 tests; 6277 PASS, 132 FAIL, 0 TIMEOUT, 0 NOTRUN.
Found 6418 tests; 6286 PASS, 132 FAIL, 0 TIMEOUT, 0 NOTRUN.
PASS [["forwarddelete",""]] "foo[]": execCommand("forwarddelete", false, "") return value
PASS [["forwarddelete",""]] "foo[]" checks for modifications to non-editable content
PASS [["forwarddelete",""]] "foo[]" compare innerHTML
......@@ -555,6 +555,15 @@ PASS [["defaultparagraphseparator","p"],["forwarddelete",""]] "foo{}<p>" queryCo
PASS [["defaultparagraphseparator","p"],["forwarddelete",""]] "foo{}<p>" queryCommandIndeterm("forwarddelete") after
PASS [["defaultparagraphseparator","p"],["forwarddelete",""]] "foo{}<p>" queryCommandState("forwarddelete") after
PASS [["defaultparagraphseparator","p"],["forwarddelete",""]] "foo{}<p>" queryCommandValue("forwarddelete") after
PASS [["forwarddelete",""]] "<table><tr><td>[]b</table>foo": execCommand("forwarddelete", false, "") return value
PASS [["forwarddelete",""]] "<table><tr><td>[]b</table>foo" checks for modifications to non-editable content
PASS [["forwarddelete",""]] "<table><tr><td>[]b</table>foo" compare innerHTML
PASS [["forwarddelete",""]] "<table><tr><td>[]b</table>foo" queryCommandIndeterm("forwarddelete") before
PASS [["forwarddelete",""]] "<table><tr><td>[]b</table>foo" queryCommandState("forwarddelete") before
PASS [["forwarddelete",""]] "<table><tr><td>[]b</table>foo" queryCommandValue("forwarddelete") before
PASS [["forwarddelete",""]] "<table><tr><td>[]b</table>foo" queryCommandIndeterm("forwarddelete") after
PASS [["forwarddelete",""]] "<table><tr><td>[]b</table>foo" queryCommandState("forwarddelete") after
PASS [["forwarddelete",""]] "<table><tr><td>[]b</table>foo" queryCommandValue("forwarddelete") after
PASS [["forwarddelete",""]] "<table><tr><td>{}</table>foo": execCommand("forwarddelete", false, "") return value
PASS [["forwarddelete",""]] "<table><tr><td>{}</table>foo" checks for modifications to non-editable content
PASS [["forwarddelete",""]] "<table><tr><td>{}</table>foo" compare innerHTML
......
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
layer at (0,0) size 800x600
LayoutView at (0,0) size 800x600
layer at (0,0) size 800x600
LayoutNGBlockFlow {HTML} at (0,0) size 800x600
LayoutNGBlockFlow {BODY} at (8,8) size 784x584
LayoutNGBlockFlow {DIV} at (0,0) size 784x241 [border: (4px solid #0000FF)]
LayoutNGBlockFlow {DIV} at (20,20) size 744x69
LayoutText {#text} at (0,0) size 59x26
text run at (0,0) width 59: "Tests:"
LayoutBR {BR} at (59,0) size 0x0
LayoutText {#text} at (0,27) size 657x20
text run at (0,27) width 657: "Our ability to \"edit around\" content the HTML editing code does not yet handle very well."
LayoutBR {BR} at (657,27) size 0x0
LayoutInline {I} at (0,0) size 99x20
LayoutText {#text} at (0,48) size 99x20
text run at (0,48) width 99: "For this test: "
LayoutText {#text} at (99,48) size 405x20
text run at (99,48) width 405: "Select and delete a table and some surrounding content."
LayoutNGBlockFlow (anonymous) at (20,89) size 744x21
LayoutBR {BR} at (0,0) size 0x0
LayoutNGBlockFlow {DIV} at (20,110) size 744x111
LayoutText {#text} at (0,0) size 183x26
text run at (0,0) width 183: "Expected Results:"
LayoutBR {BR} at (183,0) size 0x0
LayoutText {#text} at (0,27) size 709x41
text run at (0,27) width 709: "The content in the red box must exactly match the content in the green box (except for the border"
text run at (0,48) width 48: "color)."
LayoutBR {BR} at (48,48) size 0x0
LayoutInline {I} at (0,0) size 99x20
LayoutText {#text} at (0,69) size 99x20
text run at (0,69) width 99: "For this test: "
LayoutText {#text} at (99,69) size 744x41
text run at (99,69) width 645: "Only selected content should get deleted. Surrounding content that is not selected should"
text run at (0,90) width 201: "(obviously) not be affected."
LayoutNGBlockFlow {DIV} at (0,251) size 784x32 [border: (2px solid #008000)]
LayoutText {#text} at (2,2) size 62x27
text run at (2,2) width 62: "before"
LayoutBlockFlow {DIV} at (0,293) size 784x32
LayoutBlockFlow {DIV} at (0,0) size 784x32 [border: (2px solid #FF0000)]
LayoutText {#text} at (2,2) size 62x27
text run at (2,2) width 62: "before"
caret: position 7 of child 0 {#text} of child 1 {DIV} of child 5 {DIV} of body
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
layer at (0,0) size 800x600
LayoutView at (0,0) size 800x600
layer at (0,0) size 800x600
LayoutNGBlockFlow {HTML} at (0,0) size 800x600
LayoutNGBlockFlow {BODY} at (8,8) size 784x584
LayoutNGBlockFlow {DIV} at (0,0) size 784x262 [border: (4px solid #0000FF)]
LayoutNGBlockFlow {DIV} at (20,20) size 744x69
LayoutText {#text} at (0,0) size 59x26
text run at (0,0) width 59: "Tests:"
LayoutBR {BR} at (59,0) size 0x0
LayoutText {#text} at (0,27) size 657x20
text run at (0,27) width 657: "Our ability to \"edit around\" content the HTML editing code does not yet handle very well."
LayoutBR {BR} at (657,27) size 0x0
LayoutInline {I} at (0,0) size 99x20
LayoutText {#text} at (0,48) size 99x20
text run at (0,48) width 99: "For this test: "
LayoutText {#text} at (99,48) size 392x20
text run at (99,48) width 392: "Select and delete a list and some surrounding content."
LayoutNGBlockFlow (anonymous) at (20,89) size 744x21
LayoutBR {BR} at (0,0) size 0x0
LayoutNGBlockFlow {DIV} at (20,110) size 744x132
LayoutText {#text} at (0,0) size 183x26
text run at (0,0) width 183: "Expected Results:"
LayoutBR {BR} at (183,0) size 0x0
LayoutText {#text} at (0,27) size 709x41
text run at (0,27) width 709: "The content in the red box must exactly match the content in the green box (except for the border"
text run at (0,48) width 48: "color)."
LayoutBR {BR} at (48,48) size 0x0
LayoutInline {I} at (0,0) size 99x20
LayoutText {#text} at (0,69) size 99x20
text run at (0,69) width 99: "For this test: "
LayoutText {#text} at (99,69) size 744x41
text run at (99,69) width 645: "Only selected content should get deleted. Surrounding content that is not selected should"
text run at (0,90) width 201: "(obviously) not be affected."
LayoutBR {BR} at (201,90) size 0x0
LayoutInline {B} at (0,0) size 718x20
LayoutText {#text} at (0,111) size 718x20
text run at (0,111) width 718: "There is a bug: the caret ends up in the wrong position, it should be in the empty paragraph."
LayoutNGBlockFlow (anonymous) at (20,242) size 744x0
LayoutInline {B} at (0,0) size 0x0
LayoutText {#text} at (0,0) size 0x0
LayoutNGBlockFlow (anonymous) at (0,272) size 784x0
LayoutInline {B} at (0,0) size 0x0
LayoutText {#text} at (0,0) size 0x0
LayoutNGBlockFlow (anonymous) at (0,272) size 784x126
LayoutNGBlockFlow {DIV} at (0,0) size 784x58 [border: (2px solid #008000)]
LayoutBR {BR} at (2,2) size 0x0
LayoutText {#text} at (2,29) size 50x26
text run at (2,29) width 50: "after"
LayoutBlockFlow {DIV} at (0,68) size 784x58
LayoutBlockFlow {DIV} at (0,0) size 784x58 [border: (2px solid #FF0000)]
LayoutBR {BR} at (2,2) size 0x26
LayoutText {#text} at (2,29) size 50x26
text run at (2,29) width 50: "after"
LayoutNGBlockFlow (anonymous) at (0,398) size 784x0
LayoutInline {B} at (0,0) size 0x0
caret: position 0 of child 1 {#text} of child 1 {DIV} of child 3 {DIV} of child 2 {B} of body
......@@ -208,14 +208,19 @@ void TypingCommand::DeleteSelectionIfRange(const VisibleSelection& selection,
EditingState* editing_state) {
if (!selection.IsRange())
return;
ApplyCommandToComposite(DeleteSelectionCommand::Create(
selection, DeleteSelectionOptions::Builder()
.SetSmartDelete(smart_delete_)
.SetMergeBlocksAfterDelete(true)
.SetExpandForSpecialElements(true)
.SetSanitizeMarkup(true)
.Build()),
editing_state);
// Although the 'selection' to delete is indeed a Range, it may have been
// built from a Caret selection; in that case we don't want to expand so that
// the table structure is deleted as well.
bool expand_for_special = EndingSelection().IsRange();
ApplyCommandToComposite(
DeleteSelectionCommand::Create(
selection, DeleteSelectionOptions::Builder()
.SetSmartDelete(smart_delete_)
.SetMergeBlocksAfterDelete(true)
.SetExpandForSpecialElements(expand_for_special)
.SetSanitizeMarkup(true)
.Build()),
editing_state);
}
void TypingCommand::DeleteKeyPressed(Document& document,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment