Skip to content

Commit c369f68

Browse files
trflynn89gmta
authored andcommitted
LibWeb: Delete entire graphemes when the delete/backspace key is pressed
We currently delete a single code unit. If the user presses backspace on a multi code point emoji, they are going to expect the entire emoji to be removed. This now matches the behavior of Chrome and Firefox.
1 parent 76bab90 commit c369f68

File tree

8 files changed

+69
-19
lines changed

8 files changed

+69
-19
lines changed

Libraries/LibWeb/Editing/Commands.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* SPDX-License-Identifier: BSD-2-Clause
55
*/
66

7+
#include <LibUnicode/Segmenter.h>
78
#include <LibWeb/CSS/StyleValues/KeywordStyleValue.h>
89
#include <LibWeb/DOM/Comment.h>
910
#include <LibWeb/DOM/Document.h>
@@ -204,12 +205,17 @@ bool command_delete_action(DOM::Document& document, Utf16String const&)
204205
block_node_child_is_relevant_type = child_element.local_name().is_one_of(HTML::TagNames::br, HTML::TagNames::hr, HTML::TagNames::img);
205206
}
206207
}
207-
if ((is<DOM::Text>(*node) && offset != 0) || block_node_child_is_relevant_type) {
208+
209+
if (auto const* text_node = as_if<DOM::Text>(*node); (text_node && offset != 0) || block_node_child_is_relevant_type) {
210+
auto start_offset = text_node
211+
? text_node->grapheme_segmenter().previous_boundary(offset).value_or(offset - 1)
212+
: offset - 1;
213+
208214
// 1. Call collapse(node, offset) on the context object's selection.
209215
MUST(selection.collapse(node, offset));
210216

211217
// 2. Call extend(node, offset − 1) on the context object's selection.
212-
MUST(selection.extend(*node, offset - 1));
218+
MUST(selection.extend(*node, start_offset));
213219

214220
// 3. Delete the selection.
215221
delete_the_selection(selection);
@@ -927,9 +933,9 @@ bool command_forward_delete_action(DOM::Document& document, Utf16String const&)
927933
}
928934

929935
// 5. If node is a Text node and offset is not node's length:
930-
if (is<DOM::Text>(*node) && offset != node->length()) {
936+
if (auto const* text_node = as_if<DOM::Text>(*node); text_node && offset != node->length()) {
931937
// 1. Let end offset be offset plus one.
932-
auto end_offset = offset + 1;
938+
auto end_offset = text_node->grapheme_segmenter().next_boundary(offset).value_or(offset + 1);
933939

934940
// FIXME: 2. While end offset is not node's length and the end offsetth code unit of node's data has general category M
935941
// when interpreted as a Unicode code point, add one to end offset.

Libraries/LibWeb/HTML/FormAssociatedElement.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -829,13 +829,12 @@ void FormAssociatedTextControlElement::handle_delete(DeleteDirection direction)
829829

830830
if (selection_start == selection_end) {
831831
if (direction == DeleteDirection::Backward) {
832-
if (selection_start > 0)
833-
MUST(set_range_text({}, selection_start - 1, selection_end, Bindings::SelectionMode::End));
832+
if (auto offset = text_node->grapheme_segmenter().previous_boundary(m_selection_end); offset.has_value())
833+
selection_start = *offset;
834834
} else {
835-
if (selection_start < text_node->length_in_utf16_code_units())
836-
MUST(set_range_text({}, selection_start, selection_end + 1, Bindings::SelectionMode::End));
835+
if (auto offset = text_node->grapheme_segmenter().next_boundary(m_selection_end); offset.has_value())
836+
selection_end = *offset;
837837
}
838-
return;
839838
}
840839

841840
MUST(set_range_text({}, selection_start, selection_end, Bindings::SelectionMode::End));
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,6 @@
1+
--- a ---
12
Before: foobar
23
After: fobar
4+
--- b ---
5+
Before: foo👩🏼‍❤️‍👨🏻bar
6+
After: foobar

Tests/LibWeb/Text/expected/Editing/execCommand-forwardDelete.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,6 @@ After: a&nbsp; &nbsp; b
1919
--- g ---
2020
Before: &nbsp;&nbsp;b
2121
After: &nbsp;b
22+
--- h ---
23+
Before: foo👩🏼‍❤️‍👨🏻bar
24+
After: foobar
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
--- a ---
2+
Before: foo👩🏼‍❤️‍👨🏻bar
3+
After: foobar
4+
--- b ---
5+
Before: foo👩🏼‍❤️‍👨🏻bar
6+
After: foobar
Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,25 @@
11
<!DOCTYPE html>
22
<script src="../include.js"></script>
3-
<div contenteditable="true">foobar</div>
3+
<div id="a" contenteditable="true">foobar</div>
4+
<div id="b" contenteditable="true">foo👩🏼‍❤️‍👨🏻bar</div>
45
<script>
56
test(() => {
6-
var divElm = document.querySelector('div');
7-
println(`Before: ${divElm.textContent}`);
7+
const testDelete = function (divId, position) {
8+
println(`--- ${divId} ---`);
9+
const divElm = document.querySelector(`div#${divId}`);
10+
println(`Before: ${divElm.textContent}`);
811

9-
// Put cursor after 'foo'
10-
var range = document.createRange();
11-
range.setStart(divElm.childNodes[0], 3);
12-
getSelection().addRange(range);
12+
// Place cursor
13+
const node = divElm.childNodes[0];
14+
getSelection().setBaseAndExtent(node, position, node, position);
1315

14-
// Press backspace
15-
document.execCommand('delete');
16+
// Press backspace
17+
document.execCommand("delete");
1618

17-
println(`After: ${divElm.textContent}`);
19+
println(`After: ${divElm.textContent}`);
20+
};
21+
22+
testDelete("a", 3);
23+
testDelete("b", 15);
1824
});
1925
</script>

Tests/LibWeb/Text/input/Editing/execCommand-forwardDelete.html

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
<div id="e" contenteditable="true">a&nbsp;&nbsp;&nbsp;&nbsp;b</div>
88
<div id="f" contenteditable="true">a&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;b</div>
99
<div id="g" contenteditable="true">&nbsp;&nbsp;b</div>
10+
<div id="h" contenteditable="true">foo👩🏼‍❤️‍👨🏻bar</div>
1011
<script>
1112
test(() => {
1213
const testForwardDelete = function(divId, position) {
@@ -31,5 +32,6 @@
3132
testForwardDelete('e', 1);
3233
testForwardDelete('f', 1);
3334
testForwardDelete('g', 0);
35+
testForwardDelete('h', 3);
3436
});
3537
</script>
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<!DOCTYPE html>
2+
<script src="include.js"></script>
3+
<input id="a" value="foo👩🏼‍❤️‍👨🏻bar" />
4+
<input id="b" value="foo👩🏼‍❤️‍👨🏻bar" />
5+
<script>
6+
test(() => {
7+
const testDelete = function (id, position, key) {
8+
println(`--- ${id} ---`);
9+
const input = document.querySelector(`input#${id}`);
10+
println(`Before: ${input.value}`);
11+
12+
// Place cursor
13+
input.setSelectionRange(position, position);
14+
15+
// Press backspace
16+
internals.sendKey(input, key);
17+
18+
println(`After: ${input.value}`);
19+
};
20+
21+
testDelete("a", 15, "Backspace");
22+
testDelete("b", 3, "Delete");
23+
});
24+
</script>

0 commit comments

Comments
 (0)