Skip to content

Commit

Permalink
Consecutive calls to -insertTextAlternatives: should not cause dict…
Browse files Browse the repository at this point in the history
…ation underlines to disappear

https://bugs.webkit.org/show_bug.cgi?id=270370
rdar://107087527

Reviewed by Abrar Rahman Protyasha.

Add support for preserving dictation underlines when UIKit calls into `-insertTextAlternatives:`,
while finalizing dictation results. Currently, UIKit only calls into `-insertText:` with the best
alternative for the dictated text, which loses all information about dictation alternatives (along
with the blue dotted underlines). While there's public API on `UITextInput` to
`-insertDictationResults:` which we _could_ implement, the suggestions will eventually be shown by
UIKit when tapping on a dictation alternative range via `-alternativesForSelectedText`, which means
that we would need to convert `UIDictationPhrase` objects into `BETextAlternatives` — something
that's not possible without introducing new API surface in BrowserEngineKit.

Instead, UIKit will be inserting text alternatives in the form of `BETextAlternatives` (or
`NSTextAlternatives` in the case where BrowserEngineKit integration is disabled), which we'll honor
by adding dictation alternative document markers.

Test: TextAlternatives.InsertDictationResultsAsAlternatives

* Source/WebCore/editing/Editor.cpp:
(WebCore::Editor::updateMarkersForWordsAffectedByEditing):

Prevent dictation markers from being removed as a result of inserting plain text, while finalizing
dictation results.

* Source/WebCore/page/EditorClient.h:
(WebCore::EditorClient::shouldRemoveDictationAlternativesAfterEditing const):
* Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.h:
* Source/WebKit/WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm:
(WebKit::WebEditorClient::shouldRemoveDictationAlternativesAfterEditing const):
* Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::shouldRemoveDictationAlternativesAfterEditing const):
* Tools/TestWebKitAPI/Tests/ios/TextAlternatives.mm:
(TestWebKitAPI::TEST):
* Tools/TestWebKitAPI/cocoa/TestWKWebView.h:
* Tools/TestWebKitAPI/cocoa/TestWKWebView.mm:

Add an API test to exercise this change.

(-[WKWebView insertText:alternatives:]):

Canonical link: https://commits.webkit.org/275579@main
  • Loading branch information
whsieh committed Mar 2, 2024
1 parent 38b6c34 commit ee754d7
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 1 deletion.
5 changes: 4 additions & 1 deletion Source/WebCore/editing/Editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3285,13 +3285,16 @@ void Editor::updateMarkersForWordsAffectedByEditing(bool doNotRemoveIfSelectionA

OptionSet<DocumentMarker::Type> markerTypesToRemove {
DocumentMarker::Type::CorrectionIndicator,
DocumentMarker::Type::DictationAlternatives,
DocumentMarker::Type::SpellCheckingExemption,
DocumentMarker::Type::Spelling,
#if !PLATFORM(IOS_FAMILY)
DocumentMarker::Type::Grammar,
#endif
};

if (CheckedPtr client = this->client(); client && client->shouldRemoveDictationAlternativesAfterEditing())
markerTypesToRemove.add(DocumentMarker::Type::DictationAlternatives);

adjustMarkerTypesToRemoveForWordsAffectedByEditing(markerTypesToRemove);

removeMarkers(wordRange, markerTypesToRemove, RemovePartiallyOverlappingMarker::Yes);
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/page/EditorClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class EditorClient : public CanMakeWeakPtr<EditorClient>, public CanMakeCheckedP
virtual bool shouldChangeSelectedRange(const std::optional<SimpleRange>& fromRange, const std::optional<SimpleRange>& toRange, Affinity, bool stillSelecting) = 0;
virtual bool shouldRevealCurrentSelectionAfterInsertion() const { return true; };
virtual bool shouldSuppressPasswordEcho() const { return false; };
virtual bool shouldRemoveDictationAlternativesAfterEditing() const { return true; }

virtual bool shouldApplyStyle(const StyleProperties&, const std::optional<SimpleRange>&) = 0;
virtual void didApplyStyle() = 0;
Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ class WebEditorClient final : public WebCore::EditorClient, public WebCore::Text
bool shouldAllowSingleClickToChangeSelection(WebCore::Node&, const WebCore::VisibleSelection&) const final;
bool shouldRevealCurrentSelectionAfterInsertion() const final;
bool shouldSuppressPasswordEcho() const final;
bool shouldRemoveDictationAlternativesAfterEditing() const final;
#endif

void willChangeSelectionForAccessibility() final;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@
return m_page->screenIsBeingCaptured() || m_page->hardwareKeyboardIsAttached();
}

bool WebEditorClient::shouldRemoveDictationAlternativesAfterEditing() const
{
return m_page->shouldRemoveDictationAlternativesAfterEditing();
}

} // namespace WebKit

#endif // PLATFORM(IOS_FAMILY)
1 change: 1 addition & 0 deletions Source/WebKit/WebProcess/WebPage/WebPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,7 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
#endif
void willInsertFinalDictationResult();
void didInsertFinalDictationResult();
bool shouldRemoveDictationAlternativesAfterEditing() const;
void replaceDictatedText(const String& oldText, const String& newText);
void replaceSelectedText(const String& oldText, const String& newText);
void requestAutocorrectionData(const String& textForAutocorrection, CompletionHandler<void(WebAutocorrectionData)>&& reply);
Expand Down
5 changes: 5 additions & 0 deletions Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
Original file line number Diff line number Diff line change
Expand Up @@ -2652,6 +2652,11 @@ static inline bool rectIsTooBigForSelection(const IntRect& blockRect, const Loca
m_ignoreSelectionChangeScopeForDictation = makeUnique<IgnoreSelectionChangeForScope>(*frame);
}

bool WebPage::shouldRemoveDictationAlternativesAfterEditing() const
{
return !m_ignoreSelectionChangeScopeForDictation;
}

void WebPage::didInsertFinalDictationResult()
{
m_ignoreSelectionChangeScopeForDictation = nullptr;
Expand Down
17 changes: 17 additions & 0 deletions Tools/TestWebKitAPI/Tests/ios/TextAlternatives.mm
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,23 @@ - (NSUInteger)dictationAlternativesMarkerCount:(NSString *)evaluateNodeExpressio
EXPECT_EQ(0U, [webView dictationAlternativesMarkerCount:@"document.body.childNodes[0]"]);
}

TEST(TextAlternatives, InsertDictationResultsAsAlternatives)
{
auto webView = createWebViewForTestingTextAlternatives();
[webView synchronouslyLoadHTMLString:@"<body></body>"];
[webView stringByEvaluatingJavaScript:@"getSelection().setPosition(document.body)"];

[[webView textInputContentView] willInsertFinalDictationResult];
[webView insertText:@"I " alternatives:@[ ]];
[webView insertText:@"wanna" alternatives:@[ @"want to" ]];
[webView insertText:@" test" alternatives:@[ ]];
[webView insertText:@" dictation" alternatives:@[ ]];
[[webView textInputContentView] didInsertFinalDictationResult];

[webView waitForNextPresentationUpdate];
EXPECT_EQ(1U, [webView dictationAlternativesMarkerCount:@"document.body.childNodes[0]"]);
}

} // namespace TestWebKitAPI

#endif // PLATFORM(IOS_FAMILY)
1 change: 1 addition & 0 deletions Tools/TestWebKitAPI/cocoa/TestWKWebView.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ struct AutocorrectionContext {
- (std::pair<CGRect, CGRect>)autocorrectionRectsForString:(NSString *)string;
- (NSArray<_WKTextInputContext *> *)synchronouslyRequestTextInputContextsInRect:(CGRect)rect;
- (void)replaceText:(NSString *)input withText:(NSString *)correction shouldUnderline:(BOOL)shouldUnderline completion:(void(^)())completion;
- (void)insertText:(NSString *)primaryString alternatives:(NSArray<NSString *> *)alternatives;
- (void)handleKeyEvent:(WebEvent *)event completion:(void (^)(WebEvent *theEvent, BOOL handled))completion;
- (void)selectTextForContextMenuWithLocationInView:(CGPoint)locationInView completion:(void(^)(BOOL shouldPresent))completion;
- (void)defineSelection;
Expand Down
16 changes: 16 additions & 0 deletions Tools/TestWebKitAPI/cocoa/TestWKWebView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,22 @@ - (void)replaceText:(NSString *)input withText:(NSString *)correction shouldUnde
}];
}

- (void)insertText:(NSString *)primaryString alternatives:(NSArray<NSString *> *)alternativeStrings
{
if (!alternativeStrings.count) {
[self.textInputContentView insertText:primaryString];
return;
}

#if USE(BROWSERENGINEKIT)
auto nsAlternatives = adoptNS([[NSTextAlternatives alloc] initWithPrimaryString:primaryString alternativeStrings:alternativeStrings]);
auto alternatives = adoptNS([[BETextAlternatives alloc] _initWithNSTextAlternatives:nsAlternatives.get()]);
[self.asyncTextInput insertTextAlternatives:alternatives.get()];
#else
[self.textInputContentView insertText:primaryString alternatives:alternativeStrings style:UITextAlternativeStyleNone];
#endif
}

#if USE(BROWSERENGINEKIT)

static RetainPtr<BEKeyEntry> wrap(WebEvent *webEvent)
Expand Down

0 comments on commit ee754d7

Please sign in to comment.