- Revision
- 293230
- Author
- [email protected]
- Date
- 2022-04-22 10:28:55 -0700 (Fri, 22 Apr 2022)
Log Message
[iOS] `-selectWordForReplacement` should select dictation alternatives that span multiple words
https://bugs.webkit.org/show_bug.cgi?id=239622
rdar://91416535
Reviewed by Aditya Keerthi.
Source/WebKit:
Currently, `-selectWordForReplacement` (which is invoked by UIKit when the tapping on spelling corrections and
dictation alternatives to reveal the callout bar with text suggestions) handles multi-word dictation ranges by
only selecting one of the words (i.e., whichever word is closest to the tapped location). This makes it
impossible to handle dictation ranges that span multiple words, since only part of the range is replaced with
the chosen alternative.
To address this, instead of using the existing `extendSelection` method (which is still exercised when tapping
the "Select" action in the callout bar with a caret selection), we add `extendSelectionForReplacement`, which
extends the selection to include any dictation alternative range that encompasses the selection (and otherwise
falls back to selecting the current word).
Test: SelectionTests.SelectWordForReplacementWithDictationAlternative
* UIProcess/WebPageProxy.h:
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView selectWordForReplacement]):
* UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::WebPageProxy::extendSelectionForReplacement):
Add a new IPC message, `WebPage::ExtendSelectionForReplacement`, and use it to implement `-[WKContentView
selectWordForReplacement]`.
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::extendSelectionForReplacement):
Add logic to select the enclosing dictation alternative marker range (if it exists), or the current word at the
start of the selection if there is no enclosing marker range. Note that we first grab a Vector of all the marker
ranges instead of creating them as we iterate through the list of `RenderedDocumentMarker*`, since creating
visible position ranges from each marker range triggers layout, and each `RenderedDocumentMarker*` is not
ref-counted, so it's possible that a reentrant call to update the document marker controller during layout could
destroy the next `RenderedDocumentMarker*` during iteration.
(WebKit::WebPage::extendSelection):
(WebKit::WebPage::setSelectedRangeDispatchingSyntheticMouseEventsIfNeeded):
Pull out logic for dispatching synthetic mouse events when selecting text into a separate helper method, which
is now called from both `extendSelection` and `extendSelectionForReplacement`.
Tools:
Add an API test to exercise the change.
* TestWebKitAPI/Tests/ios/SelectionByWord.mm:
(TEST):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (293229 => 293230)
--- trunk/Source/WebKit/ChangeLog 2022-04-22 16:44:10 UTC (rev 293229)
+++ trunk/Source/WebKit/ChangeLog 2022-04-22 17:28:55 UTC (rev 293230)
@@ -1,3 +1,51 @@
+2022-04-22 Wenson Hsieh <[email protected]>
+
+ [iOS] `-selectWordForReplacement` should select dictation alternatives that span multiple words
+ https://bugs.webkit.org/show_bug.cgi?id=239622
+ rdar://91416535
+
+ Reviewed by Aditya Keerthi.
+
+ Currently, `-selectWordForReplacement` (which is invoked by UIKit when the tapping on spelling corrections and
+ dictation alternatives to reveal the callout bar with text suggestions) handles multi-word dictation ranges by
+ only selecting one of the words (i.e., whichever word is closest to the tapped location). This makes it
+ impossible to handle dictation ranges that span multiple words, since only part of the range is replaced with
+ the chosen alternative.
+
+ To address this, instead of using the existing `extendSelection` method (which is still exercised when tapping
+ the "Select" action in the callout bar with a caret selection), we add `extendSelectionForReplacement`, which
+ extends the selection to include any dictation alternative range that encompasses the selection (and otherwise
+ falls back to selecting the current word).
+
+ Test: SelectionTests.SelectWordForReplacementWithDictationAlternative
+
+ * UIProcess/WebPageProxy.h:
+ * UIProcess/ios/WKContentViewInteraction.mm:
+ (-[WKContentView selectWordForReplacement]):
+ * UIProcess/ios/WebPageProxyIOS.mm:
+ (WebKit::WebPageProxy::extendSelectionForReplacement):
+
+ Add a new IPC message, `WebPage::ExtendSelectionForReplacement`, and use it to implement `-[WKContentView
+ selectWordForReplacement]`.
+
+ * WebProcess/WebPage/WebPage.h:
+ * WebProcess/WebPage/WebPage.messages.in:
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::extendSelectionForReplacement):
+
+ Add logic to select the enclosing dictation alternative marker range (if it exists), or the current word at the
+ start of the selection if there is no enclosing marker range. Note that we first grab a Vector of all the marker
+ ranges instead of creating them as we iterate through the list of `RenderedDocumentMarker*`, since creating
+ visible position ranges from each marker range triggers layout, and each `RenderedDocumentMarker*` is not
+ ref-counted, so it's possible that a reentrant call to update the document marker controller during layout could
+ destroy the next `RenderedDocumentMarker*` during iteration.
+
+ (WebKit::WebPage::extendSelection):
+ (WebKit::WebPage::setSelectedRangeDispatchingSyntheticMouseEventsIfNeeded):
+
+ Pull out logic for dispatching synthetic mouse events when selecting text into a separate helper method, which
+ is now called from both `extendSelection` and `extendSelectionForReplacement`.
+
2022-04-22 Zan Dobersek <[email protected]>
[GLib] Make WebKitSettings XSS auditor functions no-op
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (293229 => 293230)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2022-04-22 16:44:10 UTC (rev 293229)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2022-04-22 17:28:55 UTC (rev 293230)
@@ -867,6 +867,7 @@
void selectWithTwoTouches(const WebCore::IntPoint from, const WebCore::IntPoint to, GestureType, GestureRecognizerState, CompletionHandler<void(const WebCore::IntPoint&, GestureType, GestureRecognizerState, OptionSet<SelectionFlags>)>&&);
void extendSelection(WebCore::TextGranularity, CompletionHandler<void()>&& = { });
void selectWordBackward();
+ void extendSelectionForReplacement(CompletionHandler<void()>&&);
void moveSelectionByOffset(int32_t offset, CompletionHandler<void()>&&);
void selectTextWithGranularityAtPoint(const WebCore::IntPoint, WebCore::TextGranularity, bool isInteractingWithFocusedElement, CompletionHandler<void()>&&);
void selectPositionAtPoint(const WebCore::IntPoint, bool isInteractingWithFocusedElement, CompletionHandler<void()>&&);
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (293229 => 293230)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2022-04-22 16:44:10 UTC (rev 293229)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2022-04-22 17:28:55 UTC (rev 293230)
@@ -7593,7 +7593,7 @@
- (void)selectWordForReplacement
{
[self beginSelectionChange];
- _page->extendSelection(WebCore::TextGranularity::WordGranularity, [weakSelf = WeakObjCPtr<WKContentView>(self)] {
+ _page->extendSelectionForReplacement([weakSelf = WeakObjCPtr<WKContentView>(self)] {
if (auto strongSelf = weakSelf.get())
[strongSelf endSelectionChange];
});
Modified: trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm (293229 => 293230)
--- trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm 2022-04-22 16:44:10 UTC (rev 293229)
+++ trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm 2022-04-22 17:28:55 UTC (rev 293230)
@@ -709,6 +709,11 @@
m_process->send(Messages::WebPage::SelectWordBackward(), m_webPageID);
}
+void WebPageProxy::extendSelectionForReplacement(CompletionHandler<void()>&& completion)
+{
+ sendWithAsyncReply(Messages::WebPage::ExtendSelectionForReplacement(), WTFMove(completion));
+}
+
void WebPageProxy::requestRectsForGranularityWithSelectionOffset(WebCore::TextGranularity granularity, uint32_t offset, CompletionHandler<void(const Vector<WebCore::SelectionGeometry>&)>&& callback)
{
if (!hasRunningProcess())
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (293229 => 293230)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2022-04-22 16:44:10 UTC (rev 293229)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2022-04-22 17:28:55 UTC (rev 293230)
@@ -791,6 +791,7 @@
void updateSelectionWithTouches(const WebCore::IntPoint&, SelectionTouch, bool baseIsStart, CompletionHandler<void(const WebCore::IntPoint&, SelectionTouch, OptionSet<SelectionFlags>)>&&);
void selectWithTwoTouches(const WebCore::IntPoint& from, const WebCore::IntPoint& to, GestureType, GestureRecognizerState, CompletionHandler<void(const WebCore::IntPoint&, GestureType, GestureRecognizerState, OptionSet<SelectionFlags>)>&&);
void extendSelection(WebCore::TextGranularity, CompletionHandler<void()>&&);
+ void extendSelectionForReplacement(CompletionHandler<void()>&&);
void selectWordBackward();
void moveSelectionByOffset(int32_t offset, CompletionHandler<void()>&&);
void selectTextWithGranularityAtPoint(const WebCore::IntPoint&, WebCore::TextGranularity, bool isInteractingWithFocusedElement, CompletionHandler<void()>&&);
@@ -1572,6 +1573,7 @@
WebCore::VisiblePosition visiblePositionInFocusedNodeForPoint(const WebCore::Frame&, const WebCore::IntPoint&, bool isInteractingWithFocusedElement);
std::optional<WebCore::SimpleRange> rangeForGranularityAtPoint(WebCore::Frame&, const WebCore::IntPoint&, WebCore::TextGranularity, bool isInteractingWithFocusedElement);
void setFocusedFrameBeforeSelectingTextAtLocation(const WebCore::IntPoint&);
+ void setSelectedRangeDispatchingSyntheticMouseEventsIfNeeded(const WebCore::SimpleRange&, WebCore::Affinity);
void dispatchSyntheticMouseEventsForSelectionGesture(SelectionTouch, const WebCore::IntPoint&);
void sendPositionInformation(InteractionInformationAtPosition&&);
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in (293229 => 293230)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in 2022-04-22 16:44:10 UTC (rev 293229)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in 2022-04-22 17:28:55 UTC (rev 293230)
@@ -70,6 +70,7 @@
SelectWithTwoTouches(WebCore::IntPoint from, WebCore::IntPoint to, enum:uint8_t WebKit::GestureType gestureType, enum:uint8_t WebKit::GestureRecognizerState gestureState) -> (WebCore::IntPoint point, enum:uint8_t WebKit::GestureType gestureType, enum:uint8_t WebKit::GestureRecognizerState gestureState, OptionSet<WebKit::SelectionFlags> selectionFlags)
ExtendSelection(enum:uint8_t WebCore::TextGranularity granularity) -> ()
SelectWordBackward()
+ ExtendSelectionForReplacement() -> ()
MoveSelectionByOffset(int32_t offset) -> ()
SelectTextWithGranularityAtPoint(WebCore::IntPoint point, enum:uint8_t WebCore::TextGranularity granularity, bool isInteractingWithFocusedElement) -> ()
SelectPositionAtBoundaryWithDirection(WebCore::IntPoint point, enum:uint8_t WebCore::TextGranularity granularity, enum:uint8_t WebCore::SelectionDirection direction, bool isInteractingWithFocusedElement) -> ()
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (293229 => 293230)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2022-04-22 16:44:10 UTC (rev 293229)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2022-04-22 17:28:55 UTC (rev 293230)
@@ -1816,6 +1816,47 @@
completionHandler(from, gestureType, gestureState, { });
}
+void WebPage::extendSelectionForReplacement(CompletionHandler<void()>&& completion)
+{
+ auto callCompletionHandlerOnExit = makeScopeExit(WTFMove(completion));
+
+ Ref frame = CheckedRef(m_page->focusController())->focusedOrMainFrame();
+ RefPtr document = frame->document();
+ if (!document)
+ return;
+
+ auto selectedRange = frame->selection().selection().range();
+ if (!selectedRange || !selectedRange->collapsed())
+ return;
+
+ VisiblePosition position = frame->selection().selection().start();
+ RefPtr container = position.deepEquivalent().containerNode();
+ if (!container)
+ return;
+
+ auto markerRanges = document->markers().markersFor(*container, { DocumentMarker::DictationAlternatives }).map([&](auto* marker) {
+ return makeSimpleRange(*container, *marker);
+ });
+
+ std::optional<SimpleRange> rangeToSelect;
+ for (auto& markerRange : markerRanges) {
+ if (contains(makeVisiblePositionRange(markerRange), position)) {
+ // In practice, dictation markers should only span a single text node, so it's sufficient to
+ // grab the first matching range (instead of taking the union of all intersecting ranges).
+ rangeToSelect = markerRange;
+ break;
+ }
+ }
+
+ if (!rangeToSelect)
+ rangeToSelect = wordRangeFromPosition(position);
+
+ if (!rangeToSelect)
+ return;
+
+ setSelectedRangeDispatchingSyntheticMouseEventsIfNeeded(*rangeToSelect, position.affinity());
+}
+
void WebPage::extendSelection(WebCore::TextGranularity granularity, CompletionHandler<void()>&& completionHandler)
{
auto callCompletionHandlerOnExit = makeScopeExit(WTFMove(completionHandler));
@@ -1830,17 +1871,23 @@
if (!wordRange)
return;
+ setSelectedRangeDispatchingSyntheticMouseEventsIfNeeded(*wordRange, position.affinity());
+}
+
+void WebPage::setSelectedRangeDispatchingSyntheticMouseEventsIfNeeded(const SimpleRange& range, Affinity affinity)
+{
+ Ref frame = CheckedRef(m_page->focusController())->focusedOrMainFrame();
IntPoint endLocationForSyntheticMouseEvents;
bool shouldDispatchMouseEvents = shouldDispatchSyntheticMouseEventsWhenModifyingSelection();
if (shouldDispatchMouseEvents) {
RefPtr view = frame->view();
- auto startLocationForSyntheticMouseEvents = view->contentsToRootView(VisiblePosition(makeDeprecatedLegacyPosition(wordRange->start)).absoluteCaretBounds()).center();
- endLocationForSyntheticMouseEvents = view->contentsToRootView(VisiblePosition(makeDeprecatedLegacyPosition(wordRange->end)).absoluteCaretBounds()).center();
+ auto startLocationForSyntheticMouseEvents = view->contentsToRootView(VisiblePosition(makeDeprecatedLegacyPosition(range.start)).absoluteCaretBounds()).center();
+ endLocationForSyntheticMouseEvents = view->contentsToRootView(VisiblePosition(makeDeprecatedLegacyPosition(range.end)).absoluteCaretBounds()).center();
dispatchSyntheticMouseEventsForSelectionGesture(SelectionTouch::Started, startLocationForSyntheticMouseEvents);
dispatchSyntheticMouseEventsForSelectionGesture(SelectionTouch::Moved, endLocationForSyntheticMouseEvents);
}
- frame->selection().setSelectedRange(wordRange, position.affinity(), WebCore::FrameSelection::ShouldCloseTyping::Yes, UserTriggered);
+ frame->selection().setSelectedRange(range, affinity, WebCore::FrameSelection::ShouldCloseTyping::Yes, UserTriggered);
if (shouldDispatchMouseEvents)
dispatchSyntheticMouseEventsForSelectionGesture(SelectionTouch::Ended, endLocationForSyntheticMouseEvents);
Modified: trunk/Tools/ChangeLog (293229 => 293230)
--- trunk/Tools/ChangeLog 2022-04-22 16:44:10 UTC (rev 293229)
+++ trunk/Tools/ChangeLog 2022-04-22 17:28:55 UTC (rev 293230)
@@ -1,3 +1,16 @@
+2022-04-22 Wenson Hsieh <[email protected]>
+
+ [iOS] `-selectWordForReplacement` should select dictation alternatives that span multiple words
+ https://bugs.webkit.org/show_bug.cgi?id=239622
+ rdar://91416535
+
+ Reviewed by Aditya Keerthi.
+
+ Add an API test to exercise the change.
+
+ * TestWebKitAPI/Tests/ios/SelectionByWord.mm:
+ (TEST):
+
2022-04-21 John Cunningham <[email protected]>
Use GCGLSpanTuple to pass buffer parameters to multidraw calls.
Modified: trunk/Tools/TestWebKitAPI/Tests/ios/SelectionByWord.mm (293229 => 293230)
--- trunk/Tools/TestWebKitAPI/Tests/ios/SelectionByWord.mm 2022-04-22 16:44:10 UTC (rev 293229)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/SelectionByWord.mm 2022-04-22 17:28:55 UTC (rev 293230)
@@ -55,6 +55,24 @@
EXPECT_WK_STREQ([webView stringByEvaluatingJavaScript:@"getSelection().toString()"], "Three");
}
+TEST(SelectionTests, SelectWordForReplacementWithDictationAlternative)
+{
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 320)]);
+ [webView synchronouslyLoadTestPageNamed:@"editable-responsive-body"];
+
+ auto contentView = [webView textInputContentView];
+ [contentView selectAll:nil];
+ [contentView insertText:@"foo bar"];
+ [webView waitForNextPresentationUpdate];
+
+ auto alternatives = adoptNS([[NSTextAlternatives alloc] initWithPrimaryString:@"foo bar" alternativeStrings:@[ @"baz" ]]);
+ [[webView textInputContentView] addTextAlternatives:alternatives.get()];
+ [[webView textInputContentView] selectWordForReplacement];
+ [webView waitForNextPresentationUpdate];
+
+ EXPECT_WK_STREQ("foo bar", [webView selectedText]);
+}
+
@interface SelectionChangeListener : NSObject <UITextInputDelegate>
@property (nonatomic) dispatch_block_t selectionWillChangeHandler;
@property (nonatomic) dispatch_block_t selectionDidChangeHandler;
