Skip to content

Commit

Permalink
[EditContext] Don't populate getTargetRanges for deletions
Browse files Browse the repository at this point in the history
In [1] the issue was raised that getTargetRanges() in beforeinput
is not interoperable across browsers for deletions. We don't want to
include non-interoperable behaviors in EditContext, so return an empty
array for getTargetRanges() when beforeinput is received for deletions
in EditContext.

We may revisit this later on and start providing a range if
interoperable behavior can be designed.

[1] w3c/input-events#146

Bug: 999184
Change-Id: Iac697f89a7c7ecee57c850e7b929e42670c10e32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4985470
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Reviewed-by: Anupam Snigdha <snianu@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1217018}
  • Loading branch information
dandclark authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent d466831 commit deb935c
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,14 @@ StaticRangeVector* RangesFromCurrentSelectionOrExtendCaret(
const LocalFrame& frame,
SelectionModifyDirection direction,
TextGranularity granularity) {
// Due to interoperability differences in getTargetRanges() when deleting
// content, we do not provide these ranges for EditContext. Developers are
// expected to compute the ranges themselves based on selection position.
// See https://github.com/w3c/input-events/issues/146.
if (frame.GetInputMethodController().GetActiveEditContext()) {
return nullptr;
}

frame.GetDocument()->UpdateStyleAndLayout(DocumentUpdateReason::kEditing);
SelectionModifier selection_modifier(
frame, frame.Selection().GetSelectionInDOMTree());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ String StyleCommands::SelectionStartCSSPropertyValue(

String StyleCommands::ValueStyle(LocalFrame& frame, CSSPropertyID property_id) {
if (frame.GetInputMethodController().GetActiveEditContext()) {
return "";
return g_empty_string;
}

frame.GetDocument()->UpdateStyleAndLayout(DocumentUpdateReason::kEditing);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,26 @@
<script>
promise_test(async function() {
const editContext = new EditContext();
const test = document.createElement("div");
document.body.appendChild(test);
const div = document.createElement("div");
document.body.appendChild(div);
let beforeInputType = null;
let beforeInputTargetRanges = null;
div.addEventListener("beforeinput", e => {
beforeInputType = e.inputType;
beforeInputTargetRanges = e.getTargetRanges().map(
staticRange => [staticRange.startOffset, staticRange.endOffset]);
});
editContext.addEventListener("textupdate", e => {
test.innerHTML = e.text;
div.innerHTML = e.text;
});
test.editContext = editContext;
test.focus();
await test_driver.send_keys(test, 'a');
assert_equals(test.innerHTML, "a");
test.remove();
div.editContext = editContext;
div.focus();
await test_driver.send_keys(div, 'a');
assert_equals(div.innerHTML, "a");
assert_equals(beforeInputType, "insertText");
assert_equals(beforeInputTargetRanges.length, 1);
assert_array_equals(beforeInputTargetRanges[0], [0, 0]);
div.remove();
}, 'Testing EditContext English typing');

promise_test(async function() {
Expand Down Expand Up @@ -60,7 +70,10 @@
div.innerText = divText;
div.editContext = new EditContext();
div.focus();

let got_before_input_event = false;
div.addEventListener("beforeinput", e => {
got_before_input_event = true;
});
let got_textupdate_event = false;
div.editContext.addEventListener("textupdate", e => {
got_textupdate_event = true;
Expand All @@ -70,9 +83,47 @@
await test_driver.send_keys(div, "a");

assert_false(got_textupdate_event, "Shouldn't have received textupdate event after editContext was detached");
assert_false(got_before_input_event, "Shouldn't have received beforeinput event after editContext was detached");

div.remove();
}, "EditContext should not receive events after being detached from element");

promise_test(async function() {
const editContext = new EditContext();
const div = document.createElement("div");
div.innerText = "hello there";
document.body.appendChild(div);
let beforeInputType = null;
let beforeInputTargetRanges = null;
div.addEventListener("beforeinput", e => {
beforeInputType = e.inputType;
beforeInputTargetRanges = e.getTargetRanges().map(
staticRange => [staticRange.startOffset, staticRange.endOffset]);
});
let textUpdateSelection = null;
editContext.addEventListener("textupdate", e => {
textUpdateSelection = [e.selectionStart, e.selectionEnd];
div.innerText = `${div.innerText.substring(0, e.updateRangeStart)}${e.text}${div.innerText.substring(e.updateRangeEnd)}`;
});
div.editContext = editContext;
editContext.updateText(0, 11, "hello there");
editContext.updateSelection(10, 10);
const selection = window.getSelection();
selection.setBaseAndExtent(div.firstChild, 10, div.firstChild, 10);

await test_driver.send_keys(div, "\uE003");
assert_equals(div.innerHTML, "hello thee");
assert_array_equals(textUpdateSelection, [9, 9]);
assert_equals(beforeInputType, "deleteContentBackward");
assert_equals(beforeInputTargetRanges.length, 0, "Backspace should not have a target range in EditContext");

await test_driver.send_keys(div, "\uE017");
assert_equals(div.innerHTML, "hello the");
assert_array_equals(textUpdateSelection, [9, 9]);
assert_equals(beforeInputType, "deleteContentForward");
assert_equals(beforeInputTargetRanges.length, 0, "Delete should not have a target range in EditContext");
div.remove();
}, "Backspace and delete in EditContext");
</script>
</body>
</html>

0 comments on commit deb935c

Please sign in to comment.