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

Backspacing shouldn't merge content into tables except for macOS

When uses hits backspace at the start of a paragraph, the content in
that block is merged in the previous block in the paragraph above. All
the browsers have the same behavior except in the case of such previous
block being a table.

Firefox and Edge both avoid merging the content into the table. I've
filed a issue in the Editing TF [1] to get some consensus, but it seems
that current behavior was implemented to match TextEdit and other native
text editing apps in macOS.

Considering that both MS Word and Google Docs avoid merging content on
backspacing, I think it's a good option for the end user that Chrome
implements that behavior as well, improving also the browsers
interoperability (we'll match Firefox and Edge).

I'll keep the current logic for macOS using an EditingBehavior flag, so
we can switch to a different behavior based on platform.

[1] https://github.com/w3c/editing/issues/164

Bug: 731000
Change-Id: Ibc5cad576b7115877a36929f717ff5ed194d0977
Reviewed-on: https://chromium-review.googlesource.com/981137
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546475}
parent d87bfe70
This tests deleting when the caret is at the start of a paragraph just after a table. The content in that paragraph should be moved into the last table cell unless that content is another table.
| <table>
| border="1"
| <tbody>
| <tr>
| <td>
| "All the content in this editable region <#selection-caret>should be in one table cell."
<!DOCTYPE html>
<html>
<body>
<p id="description">This tests deleting when the caret is at the start of a paragraph just after a table. The content in that paragraph should be moved into the last table cell unless that content is another table.</p>
<div contenteditable="true" id="root"><table border="1"><tr><td>All the content in this editable region&nbsp;</td></tr></table><div id="div">should be in one table cell.</div></div>
<script src="../../resources/dump-as-markup.js"></script>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="../assert_selection.js"></script>
<div id="log"></div>
<script>
var sel = window.getSelection();
var div = document.getElementById("div");
sel.collapse(div, 0);
document.execCommand("Delete");
Markup.description(description.textContent);
Markup.dump("root");
const isMac = navigator.platform.indexOf('Mac') === 0;
selection_test(
[
'<div contenteditable="true" id="root">',
'<table border="1">',
'<tr>',
'<td>All the content in this editable region </td>',
'</tr>',
'</table>',
'<div id="div">|should be in one table cell.</div>',
'</div>'
],
'Delete',
isMac
? [
'<div contenteditable="true" id="root">',
'<table border="1">',
'<tbody>',
'<tr>',
'<td>All the content in this editable region |should be in one table cell.</td>',
'</tr>',
'</tbody>',
'</table>',
'</div>'
]
: [
'<div contenteditable="true" id="root">',
'<table border="1">',
'<tbody>',
'<tr>',
'<td>All the content in this editable region </td>',
'</tr>',
'</tbody>',
'</table>',
'|should be in one table cell.',
'</div>'
],
'Deleting when the caret is at the start of a paragraph just after a table' +
isMac
? 'The content in the deleted paragraph should be moved into the last table cell unless that content is another table.'
: 'The content in the deleted paragraph should not be merged into the table above.');
</script>
</body>
</html>
......@@ -11,6 +11,7 @@ p {
</style>
<div id="log"></div>
<script>
const isMac = navigator.platform.indexOf('Mac') === 0;
test(() => {
assert_selection(
'<div contenteditable="true"><h1>Heading 1:</h1>^<p>|paragraph was merged.</p></div>',
......@@ -30,7 +31,11 @@ test(() => {
assert_selection(
'<div contenteditable="true"><table><tbody><tr><td>Table:</td></tr></tbody></table>^<p>|paragraph was merged.</p></div>',
'delete',
'<div contenteditable="true"><table><tbody><tr><td>Table:|paragraph was merged.</td></tr></tbody></table></div>',
'Make a paragraph into a table');
isMac
? '<div contenteditable="true"><table><tbody><tr><td>Table:|paragraph was merged.</td></tr></tbody></table></div>'
: '<div contenteditable="true"><table><tbody><tr><td>Table:</td></tr></tbody></table>|paragraph was merged.</div>',
isMac
? 'Make a paragraph into a table'
: 'Do not merge paragraph into a table');
}, 'merge into a block by backspace');
</script>
This test checks that deleting into a table works properly. When deleting three times with the cursor after the character "a" in the "after" text after the table, the "a" should be deleted, as should the "o" of "buffalo" in the last table cell.
Before
Foo baz
bar buffalfter
execDeleteCommand: <div>Before</div> <table style="border:3px solid #aaa;"> <tbody><tr> <td> Foo </td> <td> baz </td> </tr> <tr> <td> bar </td> <td> buffalo </td> </tr> </tbody></table> <span id="start">fter</span>
execDeleteCommand: <div>Before</div> <table style="border:3px solid #aaa;"> <tbody><tr> <td> Foo </td> <td> baz </td> </tr> <tr> <td> bar </td> <td> buffalofter</td></tr></tbody></table>
execDeleteCommand: <div>Before</div> <table style="border:3px solid #aaa;"> <tbody><tr> <td> Foo </td> <td> baz </td> </tr> <tr> <td> bar </td> <td> buffalfter</td></tr></tbody></table>
<!DOCTYPE html>
<html>
<body>
<p>This test checks that deleting into a table works properly. When deleting three times with the cursor after the character "a" in the "after" text after the table, the "a" should be deleted, as should the "o" of "buffalo" in the last table cell.</p>
<div contenteditable="true" id="root">
<div>Before</div>
<table style="border:3px solid #aaa;">
<tr>
<td>
Foo
</td>
<td>
baz
</td>
</tr>
<tr>
<td>
bar
</td>
<td>
buffalo
</td>
</tr>
</table>
<span id="start">after</span>
</div>
<script src=../editing.js></script>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="../assert_selection.js"></script>
<div id="log"></div>
<script>
function editingTest() {
var selection = window.getSelection();
var s = document.getElementById("start");
selection.collapse(s, 0);
execMoveSelectionForwardByCharacterCommand();
for (i = 0; i < 3; i++) {
deleteCommand();
}
}
runDumpAsTextEditingTest();
const isMac = navigator.platform.indexOf('Mac') === 0;
selection_test(
[
'<div contenteditable="true" id="root">',
'<div>Before</div>',
'<table style="border:3px solid #aaa;">',
'<tr>',
'<td>Foo</td>',
'<td>baz</td>',
'</tr>',
'<tr>',
'<td>bar</td>',
'<td>buffalo</td>',
'</tr>',
'</table>',
'<span id="start">^a|fter</span>',
'</div>'
],
selection => {
selection.document.execCommand('delete');
selection.document.execCommand('delete');
selection.document.execCommand('delete');
},
isMac
? [
'<div contenteditable="true" id="root">',
'<div>Before</div>',
'<table style="border:3px solid #aaa;">',
'<tbody>',
'<tr>',
'<td>Foo</td>',
'<td>baz</td>',
'</tr>',
'<tr>',
'<td>bar</td>',
'<td>buffal|fter</td>',
'</tr>',
'</tbody>',
'</table>',
'</div>',
]
: [
'<div contenteditable="true" id="root">',
'<div>Before</div>',
'<table style="border:3px solid #aaa;">',
'<tbody>',
'<tr>',
'<td>Foo</td>',
'<td>baz</td>',
'</tr>',
'<tr>',
'<td>bar</td>',
'<td>buffalo</td>',
'</tr>',
'</tbody>',
'</table>|fter',
'</div>',
],
'This test checks that deleting into a table works properly.' +
isMac
? 'When deleting three times with the cursor after the character "a" in the "after" text after the table, the "a" should be deleted, as should the "o" of "buffalo" in the last table cell.'
: 'When deleting three times with the cursor after the character "a" in the "after" text after the table, the "a" should be deleted; the rest of the delete commands should do nothing.');
</script>
</body>
</html>
......@@ -107,6 +107,14 @@ class CORE_EXPORT EditingBehavior {
return type_ == kEditingMacBehavior;
}
// On Mac, backspacing at the start of a blocks merges with the
// previous table block, as we do with regular blocks. On other
// platforms backspace event does nothing if the block above is a
// table, but allows mergin otherwise.
bool ShouldMergeContentWithTablesOnBackspace() const {
return type_ == kEditingMacBehavior;
}
// Support for global selections, used on platforms like the X Window
// System that treat selection as a type of clipboard.
bool SupportsGlobalSelection() const {
......
......@@ -848,8 +848,9 @@ void TypingCommand::DeleteKeyPressed(TextGranularity granularity,
return;
// If the caret is at the start of a paragraph after a table, move content
// into the last table cell.
if (IsStartOfParagraph(visible_start) &&
// into the last table cell (this is done to follows macOS' behavior).
if (frame->GetEditor().Behavior().ShouldMergeContentWithTablesOnBackspace() &&
IsStartOfParagraph(visible_start) &&
TableElementJustBefore(
PreviousPositionOf(visible_start, kCannotCrossEditingBoundary))) {
// Unless the caret is just before a table. We don't want to move a
......
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