Skip to content

Commit

Permalink
[UIAsyncTextInput] fast/events/ios/contenteditable-autocapitalize.htm…
Browse files Browse the repository at this point in the history
…l fails when using async text input

https://bugs.webkit.org/show_bug.cgi?id=265385
rdar://118835028

Reviewed by Megan Gardner.

Address this test failure (as well as some adjacent issues and cleanup). See below for more details.

* LayoutTests/fast/events/ios/contenteditable-autocapitalize-expected.txt:
* LayoutTests/fast/events/ios/contenteditable-autocapitalize.html:

Rewrite this layout test to be easier to follow by using existing `UIHelper` functions to wait for
the keyboard to show and dismiss, and `js-test.js` helper functions.

* Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView requestPreferredArrowDirectionForEditMenuWithCompletionHandler:]):

Prevent a self-induced release assertion by replacing an internal call to the legacy
`-_shouldSuppressSelectionCommands` function with the replacment, `-shouldSuppressEditMenu`.

(-[WKContentView _internalSelectTextForContextMenuWithLocationInView:completionHandler:]):
* Tools/TestRunnerShared/spi/UIKitSPIForTesting.h:

Include all of the `UIAsyncTextInput` and related headers, in this test-runner-only SPI header.

* Tools/TestWebKitAPI/cocoa/TestWKWebView.mm:
(-[TestWKWebView caretViewRectInContentCoordinates]):
(-[TestWKWebView selectionViewRectsInContentCoordinates]):

Take the opportunity to remove some old codepaths for grabbing selection/caret views that are no
longer relevant to the new selection display interaction codepaths in iOS 17+.

* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h:
* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptControllerIOS::applyAutocorrection):

Prevent another release assertion here by adopting the async text input version,
`-replaceText:withText:options:withCompletionHandler:`.

(WTR::UIScriptControllerIOS::clipSelectionViewRectToContentView const):
(WTR::UIScriptControllerIOS::selectionStartGrabberViewRect const):
(WTR::UIScriptControllerIOS::selectionEndGrabberViewRect const):
(WTR::UIScriptControllerIOS::selectionEndGrabberViewShapePathDescription const):
(WTR::UIScriptControllerIOS::selectionCaretViewRect const):
(WTR::UIScriptControllerIOS::selectionRangeViewRects const):

Prevent a release assertion by calling `-selectionClipRect` instead of `-_selectionClipRect` when
grabbing the caret or selection view rects for testing.

(WTR::UIScriptControllerIOS::selectionCaretBackgroundColor const):
(WTR::UIScriptControllerIOS::asyncTextInput const):

Add a helper method to return the `WKContentView`, as a `UIAsyncTextInput`.

(WTR::clipSelectionViewRectToContentView): Deleted.

Canonical link: https://commits.webkit.org/271162@main
  • Loading branch information
whsieh committed Nov 27, 2023
1 parent 1cf46fb commit 76b8798
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 74 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
To manually test, type 't' into the contenteditable. The 't' should not be autocapitalized.

With autocapitalize: none, the output is: "to"
With autocapitalize: sentences, the output is: "To"
With autocapitalize: characters, the output is: "TO"
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS With autocapitalize: none, the output is: "to"
PASS With autocapitalize: sentences, the output is: "To"
PASS With autocapitalize: characters, the output is: "TO"
PASS successfullyParsed is true

TEST COMPLETE

72 changes: 25 additions & 47 deletions LayoutTests/fast/events/ios/contenteditable-autocapitalize.html
Original file line number Diff line number Diff line change
@@ -1,59 +1,38 @@
<!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>
<script>
let write = message => output.innerHTML += message + "<br>";
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
}

let remainingInputEventCount = 0;
let resolveExpectedInputEvents = null;
function handleInput() {
remainingInputEventCount--;
if (resolveExpectedInputEvents && !remainingInputEventCount)
resolveExpectedInputEvents();
}

function runTestWithAutocapitalizeType(autocapitalizeType, inputEventCount)
{
editable.autocapitalize = autocapitalizeType;
remainingInputEventCount = inputEventCount;
resolveExpectedInputEvents = () => {
write(`With autocapitalize: ${editable.autocapitalize}, the output is: "${editable.textContent}"`);
editable.textContent = "";
editable.blur();
};

return new Promise(function(resolve) {
let rect = editable.getBoundingClientRect();
testRunner.runUIScript(`(function() {
uiController.didShowKeyboardCallback = function() {
uiController.typeCharacterUsingHardwareKeyboard("t", function() {
uiController.typeCharacterUsingHardwareKeyboard("o", function() {});
});
}
uiController.didHideKeyboardCallback = function() {
uiController.uiScriptComplete();
}
uiController.singleTapAtPoint(${rect.left + rect.width / 2}, ${rect.top + rect.height / 2}, function() {});
})();`, resolve);
});
}
jsTestIsAsync = true;
description("To manually test, type 't' into the contenteditable. The 't' should not be autocapitalized.");

async function runTest()
{
if (!window.testRunner || !testRunner.runUIScript)
return;

await runTestWithAutocapitalizeType("none", 2);
await runTestWithAutocapitalizeType("sentences", 2);
await runTestWithAutocapitalizeType("characters", 2);
testRunner.notifyDone();
for (const [autocapitalizeType, expectedTextContent] of [["none", "to"], ["sentences", "To"], ["characters", "TO"]]) {
editable.autocapitalize = autocapitalizeType;

await UIHelper.activateElementAndWaitForInputSession(editable);
await UIHelper.typeCharacter("t");
await UIHelper.ensurePresentationUpdate();
await UIHelper.typeCharacter("o");
await UIHelper.ensurePresentationUpdate();

if (expectedTextContent === editable.textContent)
testPassed(`With autocapitalize: ${autocapitalizeType}, the output is: "${editable.textContent}"`);
else
testFailed(`With autocapitalize: ${autocapitalizeType}, the output is: "${editable.textContent}" (expected "${expectedTextContent}")`);

editable.textContent = "";
editable.blur();
}

await UIHelper.waitForKeyboardToHide();
finishJSTest();
}
</script>
<style>
Expand All @@ -68,8 +47,7 @@
</head>

<body onload=runTest()>
<div contenteditable id="editable" oninput=handleInput()></div>
<p>To manually test, type 't' into the contenteditable. The 't' should not be autocapitalized.</p>
<div contenteditable id="editable"></div>
<div id="output"></div>
</body>

Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Original file line number Diff line number Diff line change
Expand Up @@ -5178,7 +5178,7 @@ - (void)_internalRequestTextRectsForString:(NSString *)input completion:(Functio

- (void)requestPreferredArrowDirectionForEditMenuWithCompletionHandler:(void(^)(UIEditMenuArrowDirection))completion
{
if ([self _shouldSuppressSelectionCommands]) {
if (self.shouldSuppressEditMenu) {
completion(UIEditMenuArrowDirectionAutomatic);
return;
}
Expand Down Expand Up @@ -8673,7 +8673,7 @@ - (void)_internalSelectTextForContextMenuWithLocationInView:(CGPoint)locationInV
if (!strongSelf)
return completionHandler(false, { });

if (shouldPresentMenu && ![strongSelf _shouldSuppressSelectionCommands])
if (shouldPresentMenu && ![strongSelf shouldSuppressEditMenu])
[strongSelf->_textInteractionWrapper activateSelection];

#if ENABLE(IMAGE_ANALYSIS_ENHANCEMENTS)
Expand Down
13 changes: 13 additions & 0 deletions Tools/TestRunnerShared/spi/UIKitSPIForTesting.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@
#import <UIKit/UIWindow_Private.h>
#import <UIKit/_UINavigationInteractiveTransition.h>

#if HAVE(UI_ASYNC_TEXT_INTERACTION)
#import <UIKit/UIAsyncTextInput.h>
#import <UIKit/UIAsyncTextInputClient.h>
#import <UIKit/UIAsyncTextInteraction.h>
#import <UIKit/UIKeyEventContext.h>
#endif

IGNORE_WARNINGS_BEGIN("deprecated-implementations")
#import <UIKit/UIWebBrowserView.h>
#import <UIKit/UIWebScrollView.h>
Expand Down Expand Up @@ -617,4 +624,10 @@ typedef NS_ENUM(NSUInteger, _UIClickInteractionEvent) {
- (UIPressInfo *)_pressInfoForPhysicalKeyboardEvent:(UIPhysicalKeyboardEvent *)physicalKeyboardEvent;
@end

#if HAVE(UI_ASYNC_TEXT_INTERACTION)
@protocol UIAsyncTextInput_Staging <UIAsyncTextInput>
@property (nonatomic, readonly) CGRect selectionClipRect; // Added in <rdar://118189933>.
@end
#endif

#endif // PLATFORM(IOS_FAMILY)
4 changes: 0 additions & 4 deletions Tools/TestWebKitAPI/cocoa/TestWKWebView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -796,8 +796,6 @@ - (CGRect)caretViewRectInContentCoordinates
#if HAVE(UI_TEXT_SELECTION_DISPLAY_INTERACTION)
if (auto view = self.textSelectionDisplayInteraction.cursorView; !view.hidden)
caretView = view;
#else
caretView = [self.textInputContentView valueForKeyPath:@"interactionAssistant.selectionView.caretView"];
#endif

return [caretView convertRect:caretView.bounds toView:self.textInputContentView];
Expand All @@ -810,8 +808,6 @@ - (CGRect)caretViewRectInContentCoordinates
#if HAVE(UI_TEXT_SELECTION_DISPLAY_INTERACTION)
if (auto view = self.textSelectionDisplayInteraction.highlightView; !view.hidden)
rects = view.selectionRects;
#else
rects = [self.textInputContentView valueForKeyPath:@"interactionAssistant.selectionView.rangeView.rects"];
#endif

for (UITextSelectionRect *rect in rects)
Expand Down
7 changes: 7 additions & 0 deletions Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
typedef struct CGRect CGRect;
OBJC_CLASS UITextSelectionDisplayInteraction;

@protocol UIAsyncTextInput_Staging;
@protocol UICoordinateSpace;

namespace WebCore {
Expand Down Expand Up @@ -193,6 +194,12 @@ class UIScriptControllerIOS final : public UIScriptControllerCocoa {

int64_t pasteboardChangeCount() const final;

void clipSelectionViewRectToContentView(CGRect&) const;

#if HAVE(UI_ASYNC_TEXT_INTERACTION)
id<UIAsyncTextInput_Staging> asyncTextInput() const;
#endif

#if HAVE(UI_TEXT_SELECTION_DISPLAY_INTERACTION)
UITextSelectionDisplayInteraction *textSelectionDisplayInteraction() const;
#endif
Expand Down
59 changes: 41 additions & 18 deletions Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,18 @@ static CGPoint contentOffsetBoundedIfNecessary(UIScrollView *scrollView, long x,
{
unsigned callbackID = m_context->prepareForAsyncTask(callback, CallbackTypeNonPersistent);

#if HAVE(UI_ASYNC_TEXT_INTERACTION)
if (auto asyncInput = asyncTextInput()) {
[asyncInput replaceText:toWTFString(oldString) withText:toWTFString(newString) options:0 withCompletionHandler:makeBlockPtr([this, protectedThis = Ref { *this }, callbackID](NSArray<UITextSelectionRect *> *) {
dispatch_async(dispatch_get_main_queue(), makeBlockPtr([this, protectedThis = Ref { *this }, callbackID] {
if (m_context)
m_context->asyncTaskComplete(callbackID);
}).get());
}).get()];
return;
}
#endif // HAVE(UI_ASYNC_TEXT_INTERACTION)

auto contentView = static_cast<id<UIWKInteractionViewProtocol>>(platformContentView());
[contentView applyAutocorrection:toWTFString(newString) toString:toWTFString(oldString) shouldUnderline:NO withCompletionHandler:makeBlockPtr([this, protectedThis = Ref { *this }, callbackID](UIWKAutocorrectionRects *) {
dispatch_async(dispatch_get_main_queue(), makeBlockPtr([this, protectedThis = Ref { *this }, callbackID] {
Expand Down Expand Up @@ -883,11 +895,20 @@ static CGPoint contentOffsetBoundedIfNecessary(UIScrollView *scrollView, long x,
return m_context->objectFromRect(rect);
}

static void clipSelectionViewRectToContentView(CGRect& rect, UIView *contentView)
void UIScriptControllerIOS::clipSelectionViewRectToContentView(CGRect& rect) const
{
auto contentView = platformContentView();
rect = CGRectIntersection(contentView.bounds, rect);
// The content view (a WKContentView in WebKit) is expected to implement the optional text input method -_selectionClipRect.
ASSERT([contentView respondsToSelector:@selector(_selectionClipRect)]);

#if HAVE(UI_ASYNC_TEXT_INTERACTION)
if (auto asyncInput = asyncTextInput()) {
auto selectionClipRect = asyncInput.selectionClipRect;
if (!CGRectIsNull(selectionClipRect))
rect = CGRectIntersection(selectionClipRect, rect);
return;
}
#endif

auto selectionClipRect = [(UIView <UITextInputInternal> *)contentView _selectionClipRect];
if (!CGRectIsNull(selectionClipRect))
rect = CGRectIntersection(selectionClipRect, rect);
Expand All @@ -906,7 +927,7 @@ static void clipSelectionViewRectToContentView(CGRect& rect, UIView *contentView
#endif

auto frameInContentViewCoordinates = [handleView convertRect:handleView.bounds toView:contentView];
clipSelectionViewRectToContentView(frameInContentViewCoordinates, contentView);
clipSelectionViewRectToContentView(frameInContentViewCoordinates);
auto jsContext = m_context->jsContext();
return JSValueToObject(jsContext, [JSValue valueWithObject:toNSDictionary(frameInContentViewCoordinates) inContext:[JSContext contextWithJSGlobalContextRef:jsContext]].JSValueRef, nullptr);
}
Expand All @@ -924,7 +945,7 @@ static void clipSelectionViewRectToContentView(CGRect& rect, UIView *contentView
#endif

auto frameInContentViewCoordinates = [handleView convertRect:handleView.bounds toView:contentView];
clipSelectionViewRectToContentView(frameInContentViewCoordinates, contentView);
clipSelectionViewRectToContentView(frameInContentViewCoordinates);
auto jsContext = m_context->jsContext();
return JSValueToObject(jsContext, [JSValue valueWithObject:toNSDictionary(frameInContentViewCoordinates) inContext:[JSContext contextWithJSGlobalContextRef:jsContext]].JSValueRef, nullptr);
}
Expand All @@ -936,9 +957,6 @@ static void clipSelectionViewRectToContentView(CGRect& rect, UIView *contentView
#if HAVE(UI_TEXT_SELECTION_DISPLAY_INTERACTION)
if (auto view = textSelectionDisplayInteraction().handleViews.lastObject; !isHiddenOrHasHiddenAncestor(view))
handleView = view;
#else
UIView *contentView = platformContentView();
handleView = [contentView valueForKeyPath:@"interactionAssistant.selectionView.rangeView.endGrabber"];
#endif

return JSValueToObject(m_context->jsContext(), [JSValue valueWithObject:toNSDictionary((CGPathRef)[handleView valueForKeyPath:@"stemView.shapeLayer.path"]).get() inContext:[JSContext contextWithJSGlobalContextRef:m_context->jsContext()]].JSValueRef, nullptr);
Expand All @@ -952,12 +970,10 @@ static void clipSelectionViewRectToContentView(CGRect& rect, UIView *contentView
#if HAVE(UI_TEXT_SELECTION_DISPLAY_INTERACTION)
if (auto view = textSelectionDisplayInteraction().cursorView; !isHiddenOrHasHiddenAncestor(view))
caretView = view;
#else
caretView = [contentView valueForKeyPath:@"interactionAssistant.selectionView.caretView"];
#endif

auto contentRect = CGRectIntersection([caretView convertRect:caretView.bounds toView:contentView], contentView.bounds);
clipSelectionViewRectToContentView(contentRect, contentView);
clipSelectionViewRectToContentView(contentRect);
if (coordinateSpace != contentView)
contentRect = [coordinateSpace convertRect:contentRect fromCoordinateSpace:contentView];
return JSValueToObject(m_context->jsContext(), [JSValue valueWithObject:toNSDictionary(contentRect) inContext:[JSContext contextWithJSGlobalContextRef:m_context->jsContext()]].JSValueRef, nullptr);
Expand Down Expand Up @@ -987,14 +1003,11 @@ static void clipSelectionViewRectToContentView(CGRect& rect, UIView *contentView
textRectInfoArray = view.selectionRects;
}
}
#else
rangeView = [contentView valueForKeyPath:@"interactionAssistant.selectionView.rangeView"];
textRectInfoArray = [rangeView valueForKeyPath:@"rects"];
#endif

for (UITextSelectionRect *textRectInfo in textRectInfoArray) {
auto rangeRectInContentViewCoordinates = [rangeView convertRect:textRectInfo.rect toView:contentView];
clipSelectionViewRectToContentView(rangeRectInContentViewCoordinates, contentView);
clipSelectionViewRectToContentView(rangeRectInContentViewCoordinates);
[rectsAsDictionaries addObject:toNSDictionary(CGRectIntersection(rangeRectInContentViewCoordinates, contentView.bounds))];
}
return JSValueToObject(m_context->jsContext(), [JSValue valueWithObject:rectsAsDictionaries.get() inContext:[JSContext contextWithJSGlobalContextRef:m_context->jsContext()]].JSValueRef, nullptr);
Expand Down Expand Up @@ -1378,9 +1391,6 @@ static UIDeviceOrientation toUIDeviceOrientation(DeviceOrientation orientation)
#if HAVE(UI_TEXT_SELECTION_DISPLAY_INTERACTION)
if (auto view = textSelectionDisplayInteraction().cursorView; !isHiddenOrHasHiddenAncestor(view))
backgroundColor = view.tintColor;
#else
auto contentView = platformContentView();
backgroundColor = [contentView valueForKeyPath:@"textInteractionAssistant.selectionView.caretView.backgroundColor"];
#endif

if (!backgroundColor)
Expand Down Expand Up @@ -1529,6 +1539,19 @@ static UIDeviceOrientation toUIDeviceOrientation(DeviceOrientation orientation)
[webView() resignFirstResponder];
}

#if HAVE(UI_ASYNC_TEXT_INTERACTION)

id<UIAsyncTextInput_Staging> UIScriptControllerIOS::asyncTextInput() const
{
static BOOL conformsToAsyncTextInput = class_conformsToProtocol(NSClassFromString(@"WKContentView"), @protocol(UIAsyncTextInput));
if (!conformsToAsyncTextInput)
return nil;

return (id<UIAsyncTextInput_Staging>)platformContentView();
}

#endif // HAVE(UI_ASYNC_TEXT_INTERACTION)

#if HAVE(UI_TEXT_SELECTION_DISPLAY_INTERACTION)

UITextSelectionDisplayInteraction *UIScriptControllerIOS::textSelectionDisplayInteraction() const
Expand Down

0 comments on commit 76b8798

Please sign in to comment.