Skip to content

Commit

Permalink
[iOS] Tap to revert does not work for multi-word autocorrections
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259522
rdar://112908387

Reviewed by Wenson Hsieh.

There are two issues preventing multi-word autocorrection UI from working correctly:

1. The correction indicator is only applied to the last word.
2. `-[WKContentView selectWordForReplacement]` does not extend the selection to cover multiple autocorrected words.

To fix (1), apply the correction indicator marker to the entire autocorrected
range. This is achieved by searching backwords for the inserted text to obtain
a range.

Then (2) is fixed by leveraging the same logic to extend the selection for
dictation alternatives, by extending the selection to cover the entire
DocumentMarker::CorrectionIndicator.

* Source/WebCore/editing/TextIterator.h:
* Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::adjustCandidateAutocorrectionInFrame):

Search for the autocorrected range, beginning at the start of the editable
content and ending at the current selection. Search backwards since we know the
range will be at the end. Mark the entire range with
`DocumentMarker::CorrectionIndicator`.

(WebKit::WebPage::extendSelectionForReplacement):

Extend the selection to encompass the entire `DocumentMarker::CorrectionIndicator`.

(WebKit::WebPage::applyAutocorrectionInternal):
* Tools/TestWebKitAPI/Tests/ios/AutocorrectionTestsIOS.mm:
(TEST):

Canonical link: https://commits.webkit.org/266336@main
  • Loading branch information
pxlcoder committed Jul 26, 2023
1 parent c0fc79a commit b422a1c
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 10 deletions.
2 changes: 1 addition & 1 deletion Source/WebCore/editing/TextIterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ WEBCORE_EXPORT bool hasAnyPlainText(const SimpleRange&, TextIteratorBehaviors =
WEBCORE_EXPORT String plainTextReplacingNoBreakSpace(const SimpleRange&, TextIteratorBehaviors = { }, bool isDisplayString = false);

// Find within the document, based on the text from the text iterator.
SimpleRange findPlainText(const SimpleRange&, const String&, FindOptions);
WEBCORE_EXPORT SimpleRange findPlainText(const SimpleRange&, const String&, FindOptions);
WEBCORE_EXPORT SimpleRange findClosestPlainText(const SimpleRange&, const String&, FindOptions, uint64_t targetCharacterOffset);
bool containsPlainText(const String& document, const String&, FindOptions); // Lets us use the search algorithm on a string.
WEBCORE_EXPORT String foldQuoteMarks(const String&);
Expand Down
24 changes: 16 additions & 8 deletions Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
Original file line number Diff line number Diff line change
Expand Up @@ -201,15 +201,23 @@ static String plainTextForDisplay(const std::optional<SimpleRange>& range)
return range ? plainTextForDisplay(*range) : emptyString();
}

static void adjustCandidateAutocorrectionInFrame(LocalFrame& frame)
static void adjustCandidateAutocorrectionInFrame(const String& correction, LocalFrame& frame)
{
#if HAVE(AUTOCORRECTION_ENHANCEMENTS)
FrameSelection selection;
selection.setSelection(frame.selection().selection());
selection.modify(FrameSelection::Alteration::Extend, SelectionDirection::Backward, TextGranularity::WordGranularity);
auto startPosition = frame.selection().selection().start();
auto endPosition = frame.selection().selection().end();

if (auto correctedRange = selection.selection().toNormalizedRange())
addMarker(*correctedRange, WebCore::DocumentMarker::CorrectionIndicator);
auto firstPositionInEditableContent = startOfEditableContent(startPosition);

auto referenceRange = makeSimpleRange(firstPositionInEditableContent, endPosition);
if (!referenceRange)
return;

auto correctedRange = findPlainText(*referenceRange, correction, { Backwards });
if (correctedRange.collapsed())
return;

addMarker(correctedRange, WebCore::DocumentMarker::CorrectionIndicator);
#else
UNUSED_PARAM(frame);
#endif
Expand Down Expand Up @@ -1883,7 +1891,7 @@ static bool insideImageOverlay(const VisiblePosition& position)
if (!container)
return;

auto markerRanges = document->markers().markersFor(*container, { DocumentMarker::DictationAlternatives }).map([&](auto* marker) {
auto markerRanges = document->markers().markersFor(*container, { DocumentMarker::DictationAlternatives, DocumentMarker::CorrectionIndicator }).map([&](auto* marker) {
return makeSimpleRange(*container, *marker);
});

Expand Down Expand Up @@ -2706,7 +2714,7 @@ static inline bool rectIsTooBigForSelection(const IntRect& blockRect, const Loca
if (correction.length()) {
frame->editor().insertText(correction, 0, originalText.isEmpty() ? TextEventInputKeyboard : TextEventInputAutocompletion);
if (isCandidate)
adjustCandidateAutocorrectionInFrame(frame.get());
adjustCandidateAutocorrectionInFrame(correction, frame.get());
} else if (originalText.length())
frame->editor().deleteWithDirection(SelectionDirection::Backward, TextGranularity::CharacterGranularity, false, true);
return true;
Expand Down
42 changes: 41 additions & 1 deletion Tools/TestWebKitAPI/Tests/ios/AutocorrectionTestsIOS.mm
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ static void checkCGRectIsEqualToCGRectWithLogging(CGRect expected, CGRect observ
}

#if HAVE(AUTOCORRECTION_ENHANCEMENTS)

TEST(AutocorrectionTests, AutocorrectionIndicatorsDismissAfterNextWord)
{
auto configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];
Expand Down Expand Up @@ -187,7 +188,46 @@ static void checkCGRectIsEqualToCGRectWithLogging(CGRect expected, CGRect observ
NSString *hasCorrectionIndicatorMarker = [webView stringByEvaluatingJavaScript:hasCorrectionIndicatorMarkerJavaScript];
EXPECT_WK_STREQ("0", hasCorrectionIndicatorMarker);
}
#endif

TEST(AutocorrectionTests, AutocorrectionIndicatorsMultiWord)
{
auto configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];

auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 568) configuration:configuration]);
auto inputDelegate = adoptNS([[TestInputDelegate alloc] init]);
[inputDelegate setFocusStartsInputSessionPolicyHandler:[] (WKWebView *, id<_WKFocusedElementInfo>) -> _WKFocusStartsInputSessionPolicy {
return _WKFocusStartsInputSessionPolicyAllow;
}];

[webView _setInputDelegate:inputDelegate.get()];
[webView synchronouslyLoadTestPageNamed:@"autofocused-text-input"];

auto *contentView = [webView textInputContentView];
[contentView insertText:@"tomorrownight"];

[webView waitForNextPresentationUpdate];

NSString *hasCorrectionIndicatorMarkerJavaScript = @"internals.hasCorrectionIndicatorMarker(0, 14);";

__block bool done = false;

[webView applyAutocorrection:@"tomorrow night" toString:@"tomorrownight" isCandidate:YES withCompletionHandler:^{
NSString *hasCorrectionIndicatorMarker = [webView stringByEvaluatingJavaScript:hasCorrectionIndicatorMarkerJavaScript];
EXPECT_WK_STREQ("1", hasCorrectionIndicatorMarker);
done = true;
}];

TestWebKitAPI::Util::run(&done);

EXPECT_WK_STREQ("", [webView selectedText]);

[contentView selectWordForReplacement];
[webView waitForNextPresentationUpdate];

EXPECT_WK_STREQ("tomorrow night", [webView selectedText]);
}

#endif // HAVE(AUTOCORRECTION_ENHANCEMENTS)

TEST(AutocorrectionTests, AutocorrectionContextBeforeAndAfterEditing)
{
Expand Down

0 comments on commit b422a1c

Please sign in to comment.