Skip to content

Commit

Permalink
REGRESSION (275503@main): Autocorrection candidates fail to insert te…
Browse files Browse the repository at this point in the history
…xt when the selection is collapsed

https://bugs.webkit.org/show_bug.cgi?id=271288
rdar://125034366

Reviewed by Richard Robinson.

The change in 275503@main refactored the text search logic in `WebPage::applyAutocorrectionInternal`
to use `rangeExpandedAroundPositionByCharacters` and `findPlainText`. However, this missed one
subtlety in the previous implementation, which is that it would set the `range` to the current
selection, in the case where both:

1. The current selection is collapsed, and
2. The `originalText` is also the empty string.

This scenario is exercised simply by inserting content by pressing one of the predictive text
candidates above the software keyboard on iOS, when the selection is after a space following a
previous word — for example:

```
Hello |(world)
```

...where `(world)` represents the predictive text candidate. The new codepath always fails here,
since `findPlainText` will be given an empty range (which it returns), and then we fail the
subsequent `!isCollapsed` check (which only makes sense in the case where the original text is non-
empty).

To fix this, we simply avoid falling down this `rangeExpandedAroundPositionByCharacters` codepath in
the first place when the selection is collapsed, and just insert the text candidate.

* LayoutTests/fast/events/ios/autocorrect-with-caret-selection.html: Added.
* LayoutTests/fast/events/ios/autocorrect-with-caret-selection-expected.txt: Added.

Add a layout test to exercise the change.

* Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::applyAutocorrectionInternal):

Canonical link: https://commits.webkit.org/276385@main
  • Loading branch information
whsieh committed Mar 20, 2024
1 parent cc094b8 commit e9f9c8b
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
To manually test, focus the editable element above, move the selection to the end, and select any text suggestion that appears in the keyboard. The text suggestion should be inserted.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS editable.textContent is "Hello world"
PASS successfullyParsed is true

TEST COMPLETE
Hello world
34 changes: 34 additions & 0 deletions LayoutTests/fast/events/ios/autocorrect-with-caret-selection.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
<html>
<head>
<meta name="viewport" content="initial-scale=1.0, user-scalable=no">
<script src="../../../resources/ui-helper.js"></script>
<script src="../../../resources/js-test.js"></script>
<style>
#editable {
border: 1px solid tomato;
display: inline-block;
font-size: 32px;
}
</style>
<script>
jsTestIsAsync = true;
addEventListener("load", async () => {
description("To manually test, focus the editable element above, move the selection to the end, and select any text suggestion that appears in the keyboard. The text suggestion should be inserted.");

editable = document.getElementById("editable");
await UIHelper.setHardwareKeyboardAttached(false);
await UIHelper.activateElementAndWaitForInputSession(editable);

getSelection().setPosition(editable, 1);
await UIHelper.applyAutocorrection("world", "");

shouldBeEqualToString("editable.textContent", "Hello world");
finishJSTest();
});
</script>
</head>
<body>
<div contenteditable id="editable">Hello&nbsp;</div>
</body>
</html>
6 changes: 5 additions & 1 deletion Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
Original file line number Diff line number Diff line change
Expand Up @@ -2774,7 +2774,11 @@ static inline bool rectIsTooBigForSelection(const IntRect& blockRect, const Loca
// forward such that it matches the original selection as much as possible.
if (foldQuoteMarks(textForRange) != originalTextWithFoldedQuoteMarks) {
// Search for the original text near the selection caret.
if (auto searchRange = rangeExpandedAroundPositionByCharacters(position, numGraphemeClusters(originalText))) {
auto characterCount = numGraphemeClusters(originalText);
if (!characterCount) {
textForRange = emptyString();
range = makeSimpleRange(position);
} else if (auto searchRange = rangeExpandedAroundPositionByCharacters(position, characterCount)) {
if (auto foundRange = findPlainText(*searchRange, originalTextWithFoldedQuoteMarks, { DoNotSetSelection, DoNotRevealSelection }); !foundRange.collapsed()) {
textForRange = plainTextForContext(foundRange);
range = foundRange;
Expand Down

0 comments on commit e9f9c8b

Please sign in to comment.