[iOS] Can't select text after dismissing the keyboard when changing focus
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Oct 2018 19:47:03 +0000 (19:47 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Oct 2018 19:47:03 +0000 (19:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190563
<rdar://problem/44613559>

Reviewed by Tim Horton.

Source/WebKit:

In r230686, we switched from using UIWKSelectionAssistant to UIWKTextInteractionAssistant for handling selection
in non-editable content on iOS; as such, when an editable element loses focus, instead of switching from the
text interaction assistant to the web selection assistant as we've previously done, we now reset our text
interaction assistant by calling `-[UIWKTextInteractionAssistant setGestureRecognizers]`, which removes all of
the current text selection gesture recognizers from WKContentView and regenerates them by building up a tree of
`UITextInteraction`s and adding them to the assistant (see `-[UITextInteractionAssistant
addGestureRecognizersToView:]`). In particular, `_UITextSelectionForceGesture` is the gesture recognizer used to
trigger text selection when long pressing.

After dismissing the keyboard by tapping the "Done" button, the UITextInteractions and gesture recognizers on
the interaction assistant include:

    <UITextInteraction>
        …
        <UITextIndirectNonEditableInteraction>
            <_UIKeyboardBasedNonEditableTextSelectionInteraction>
              ↳ "_UIKeyboardTextSelectionGestureForcePress" → <_UITextSelectionForceGesture>

However, after the keyboard dismisses due to an editable element losing focus, the UITextInteractions on the
interaction assistant look like this:

    <UITextInteraction>
        …
        <UITextIndirectNonEditableInteraction>

Subsequently, the lack of a `_UIKeyboardBasedNonEditableTextSelectionInteraction` makes text selection by long
pressing impossible, since the `_UITextSelectionForceGesture` is never introduced to `WKContentView`. In UIKit,
`UITextIndirectNonEditableInteraction` only adds `_UIKeyboardBasedNonEditableTextSelectionInteraction` as a
child if the text input view — in our case, WKContentView — is missing an input delegate (see `-initWithView:`).
In the case where the Done button is used to dismiss the keyboard, WKContentView loses first responder, and the
input delegate of WKContentView is cleared out early on, before we call `-stopAssistingKeyboard`:

    -[WKContentView(WKInteraction) setInputDelegate:]
    -[UIKeyboardImpl setDelegate:force:]
    -[UIPeripheralHost(UIKitInternal) _reloadInputViewsForResponder:]
    -[UIResponder _finishResignFirstResponder]
    -[UIResponder resignFirstResponder]
    -[WKContentView(WKInteraction) resignFirstResponderForWebView]
    -[UIKeyboardImpl dismissKeyboard]

However, in the case where the focused element is blurred, we end up clearing out the delegate in
`-_stopAssistingNode`, *after* we've already called `-setGestureRecognizers` on the interaction assistant. This
means UIKit will skip adding `_UIKeyboardBasedNonEditableTextSelectionInteraction` to the text interaction
assistant.

    -[WKContentView(WKInteraction) setInputDelegate:]
    -[UIKeyboardImpl setDelegate:force:]
    -[UIPeripheralHost(UIKitInternal) _reloadInputViewsForResponder:]
    -[UIResponder(UIResponderInputViewAdditions) reloadInputViews]
    -[WKContentView(WKInteraction) _stopAssistingNode]

To fix this, we simply reset our `inputDelegate` earlier in `_stopAssistingKeyboard` instead of waiting until
we reload input views. This ensures that UIKit sets up the text interaction assistant's gestures when changing
focus in the same way as we would when the keyboard is dismissed via `-resignFirstResponder` (e.g. when pressing
the Done button).

Test: editing/selection/ios/select-text-after-changing-focus.html

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView setupInteraction]):
(-[WKContentView setUpTextSelectionAssistant]):
(-[WKContentView _startAssistingKeyboard]):
(-[WKContentView _stopAssistingKeyboard]):
(-[WKContentView useSelectionAssistantWithGranularity:]): Deleted.

Additionally rename this to -setUpTextSelectionAssistant and remove the selection granularity argument. This was
previously used to switch between web and text interaction assistants.

Tools:

* DumpRenderTree/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptController::isShowingKeyboard const):

Add a new UIScriptController method that returns whether the keyboard is shown. See `ui-helper.js` for more
details.

* TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
* TestRunnerShared/UIScriptContext/UIScriptController.cpp:
(WTR::UIScriptController::isShowingKeyboard const):
* TestRunnerShared/UIScriptContext/UIScriptController.h:
* WebKitTestRunner/cocoa/TestRunnerWKWebView.h:
* WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:

Also rename the `isShowingKeyboard` Objective-C property to the more canonical `showingKeyboard`, with
`isShowingKeyboard` as the getter method.

(-[TestRunnerWKWebView _invokeShowKeyboardCallbackIfNecessary]):
(-[TestRunnerWKWebView _invokeHideKeyboardCallbackIfNecessary]):
* WebKitTestRunner/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptController::isShowingKeyboard const):

LayoutTests:

Add a new layout test to check that the user can make a selection by long pressing after the keyboard is
dismissed due to changing the focused element.

* editing/selection/ios/select-text-after-changing-focus-expected.txt: Added.
* editing/selection/ios/select-text-after-changing-focus.html: Added.
* resources/ui-helper.js:

Also tweak the behavior of `UIHelper.waitForKeyboardToHide()`, so that it resolves immediately if the keyboard
is not shown. This allows us to ensure that tests which use `UIHelper.waitForKeyboardToHide()` are robust in the
case where they wait for another action to complete (e.g. a simulated tap) prior to registering a keyboard
hiding callback.

(window.UIHelper.waitForKeyboardToHide.return.new.Promise):
(window.UIHelper.waitForKeyboardToHide):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@237135 268f45cc-cd09-0410-ab3c-d52691b4dbfc

14 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/selection/ios/select-text-after-changing-focus-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/ios/select-text-after-changing-focus.html [new file with mode: 0644]
LayoutTests/resources/ui-helper.js
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Tools/ChangeLog
Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h
Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm
Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm

index b4688fc..ef43ab9 100644 (file)
@@ -1,3 +1,26 @@
+2018-10-15  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Can't select text after dismissing the keyboard when changing focus
+        https://bugs.webkit.org/show_bug.cgi?id=190563
+        <rdar://problem/44613559>
+
+        Reviewed by Tim Horton.
+
+        Add a new layout test to check that the user can make a selection by long pressing after the keyboard is
+        dismissed due to changing the focused element.
+
+        * editing/selection/ios/select-text-after-changing-focus-expected.txt: Added.
+        * editing/selection/ios/select-text-after-changing-focus.html: Added.
+        * resources/ui-helper.js:
+
+        Also tweak the behavior of `UIHelper.waitForKeyboardToHide()`, so that it resolves immediately if the keyboard
+        is not shown. This allows us to ensure that tests which use `UIHelper.waitForKeyboardToHide()` are robust in the
+        case where they wait for another action to complete (e.g. a simulated tap) prior to registering a keyboard
+        hiding callback.
+
+        (window.UIHelper.waitForKeyboardToHide.return.new.Promise):
+        (window.UIHelper.waitForKeyboardToHide):
+
 2018-10-15  Andy Estes  <aestes@apple.com>
 
         [Apple Pay] Payment authorization results with ApplePayErrors should never be considered final
diff --git a/LayoutTests/editing/selection/ios/select-text-after-changing-focus-expected.txt b/LayoutTests/editing/selection/ios/select-text-after-changing-focus-expected.txt
new file mode 100644 (file)
index 0000000..6213227
--- /dev/null
@@ -0,0 +1,4 @@
+
+WebKit
+The selected text is: "WebKit"
+This test verifies that it's possible to select text by long pressing after the keyboard is dismissed due to the focused element changing. To manually test, tap the input to show the keyboard then click the button to dismiss the keyboard, and finally try to select the word 'WebKit'.
diff --git a/LayoutTests/editing/selection/ios/select-text-after-changing-focus.html b/LayoutTests/editing/selection/ios/select-text-after-changing-focus.html
new file mode 100644 (file)
index 0000000..b310620
--- /dev/null
@@ -0,0 +1,54 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<head>
+    <script src="../../../resources/basic-gestures.js"></script>
+    <script src="../../../resources/ui-helper.js"></script>
+    <style>
+    body {
+        margin: 0;
+    }
+
+    button, input {
+        width: 100%;
+        height: 100px;
+        font-size: 60px;
+        display: block;
+    }
+
+    #select {
+        font-size: 100px;
+    }
+    </style>
+    <script>
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+    }
+
+    async function run()
+    {
+        document.addEventListener("selectionchange", () => result.textContent = getSelection().toString());
+        if (!window.testRunner)
+            return;
+
+        await UIHelper.activateAndWaitForInputSessionAt(50, 50);
+        await UIHelper.tapAt(50, 150);
+        await UIHelper.waitForKeyboardToHide();
+        await longPressAtPoint(50, 250);
+        await liftUpAtPoint(50, 250);
+
+        testRunner.notifyDone();
+    }
+    </script>
+</head>
+<body onload="run()">
+    <input></input>
+    <button></button>
+    <div id="select">WebKit</div>
+    <pre>The selected text is: "<span id="result"></span>"</pre>
+    <p>This test verifies that it's possible to select text by long pressing after the keyboard is dismissed due to the
+    focused element changing. To manually test, tap the input to show the keyboard then click the button to dismiss the
+    keyboard, and finally try to select the word 'WebKit'.</p>
+</body>
+</html>
index ff0c60c..ac0d56f 100644 (file)
@@ -135,7 +135,10 @@ window.UIHelper = class UIHelper {
         return new Promise(resolve => {
             testRunner.runUIScript(`
                 (function() {
-                    uiController.didHideKeyboardCallback = () => uiController.uiScriptComplete();
+                    if (uiController.isShowingKeyboard)
+                        uiController.didHideKeyboardCallback = () => uiController.uiScriptComplete();
+                    else
+                        uiController.uiScriptComplete();
                 })()`, resolve);
         });
     }
index dc97695..39a48b9 100644 (file)
@@ -1,3 +1,79 @@
+2018-10-15  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Can't select text after dismissing the keyboard when changing focus
+        https://bugs.webkit.org/show_bug.cgi?id=190563
+        <rdar://problem/44613559>
+
+        Reviewed by Tim Horton.
+
+        In r230686, we switched from using UIWKSelectionAssistant to UIWKTextInteractionAssistant for handling selection
+        in non-editable content on iOS; as such, when an editable element loses focus, instead of switching from the
+        text interaction assistant to the web selection assistant as we've previously done, we now reset our text
+        interaction assistant by calling `-[UIWKTextInteractionAssistant setGestureRecognizers]`, which removes all of
+        the current text selection gesture recognizers from WKContentView and regenerates them by building up a tree of
+        `UITextInteraction`s and adding them to the assistant (see `-[UITextInteractionAssistant
+        addGestureRecognizersToView:]`). In particular, `_UITextSelectionForceGesture` is the gesture recognizer used to
+        trigger text selection when long pressing.
+
+        After dismissing the keyboard by tapping the "Done" button, the UITextInteractions and gesture recognizers on
+        the interaction assistant include:
+
+            <UITextInteraction>
+                …
+                <UITextIndirectNonEditableInteraction>
+                    <_UIKeyboardBasedNonEditableTextSelectionInteraction>
+                      ↳ "_UIKeyboardTextSelectionGestureForcePress" → <_UITextSelectionForceGesture>
+
+        However, after the keyboard dismisses due to an editable element losing focus, the UITextInteractions on the
+        interaction assistant look like this:
+
+            <UITextInteraction>
+                …
+                <UITextIndirectNonEditableInteraction>
+
+        Subsequently, the lack of a `_UIKeyboardBasedNonEditableTextSelectionInteraction` makes text selection by long
+        pressing impossible, since the `_UITextSelectionForceGesture` is never introduced to `WKContentView`. In UIKit,
+        `UITextIndirectNonEditableInteraction` only adds `_UIKeyboardBasedNonEditableTextSelectionInteraction` as a
+        child if the text input view — in our case, WKContentView — is missing an input delegate (see `-initWithView:`).
+        In the case where the Done button is used to dismiss the keyboard, WKContentView loses first responder, and the
+        input delegate of WKContentView is cleared out early on, before we call `-stopAssistingKeyboard`:
+
+            -[WKContentView(WKInteraction) setInputDelegate:]
+            -[UIKeyboardImpl setDelegate:force:]
+            -[UIPeripheralHost(UIKitInternal) _reloadInputViewsForResponder:]
+            -[UIResponder _finishResignFirstResponder]
+            -[UIResponder resignFirstResponder]
+            -[WKContentView(WKInteraction) resignFirstResponderForWebView]
+            -[UIKeyboardImpl dismissKeyboard]
+
+        However, in the case where the focused element is blurred, we end up clearing out the delegate in
+        `-_stopAssistingNode`, *after* we've already called `-setGestureRecognizers` on the interaction assistant. This
+        means UIKit will skip adding `_UIKeyboardBasedNonEditableTextSelectionInteraction` to the text interaction
+        assistant.
+
+            -[WKContentView(WKInteraction) setInputDelegate:]
+            -[UIKeyboardImpl setDelegate:force:]
+            -[UIPeripheralHost(UIKitInternal) _reloadInputViewsForResponder:]
+            -[UIResponder(UIResponderInputViewAdditions) reloadInputViews]
+            -[WKContentView(WKInteraction) _stopAssistingNode]
+
+        To fix this, we simply reset our `inputDelegate` earlier in `_stopAssistingKeyboard` instead of waiting until
+        we reload input views. This ensures that UIKit sets up the text interaction assistant's gestures when changing
+        focus in the same way as we would when the keyboard is dismissed via `-resignFirstResponder` (e.g. when pressing
+        the Done button).
+
+        Test: editing/selection/ios/select-text-after-changing-focus.html
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView setupInteraction]):
+        (-[WKContentView setUpTextSelectionAssistant]):
+        (-[WKContentView _startAssistingKeyboard]):
+        (-[WKContentView _stopAssistingKeyboard]):
+        (-[WKContentView useSelectionAssistantWithGranularity:]): Deleted.
+
+        Additionally rename this to -setUpTextSelectionAssistant and remove the selection granularity argument. This was
+        previously used to switch between web and text interaction assistants.
+
 2018-10-15  Remy Demarest  <rdemarest@apple.com>
 
         Web Inspector: RDM: Toolbar hidden in when Inspector is docked to side.
index 10dd333..cfe7924 100644 (file)
@@ -700,7 +700,7 @@ static inline bool hasAssistedNode(WebKit::AssistedNodeInformation assistedNodeI
     _showingTextStyleOptions = NO;
 
     // FIXME: This should be called when we get notified that loading has completed.
-    [self useSelectionAssistantWithGranularity:_webView._selectionGranularity];
+    [self setUpTextSelectionAssistant];
     
     _actionSheetAssistant = adoptNS([[WKActionSheetAssistant alloc] initWithView:self]);
     [_actionSheetAssistant setDelegate:self];
@@ -2021,14 +2021,12 @@ static void cancelPotentialTapIfNecessary(WKContentView* contentView)
     _page->handleTap(location, _layerTreeTransactionIdAtLastTouchStart);
 }
 
-- (void)useSelectionAssistantWithGranularity:(WKSelectionGranularity)selectionGranularity
+- (void)setUpTextSelectionAssistant
 {
-    _webSelectionAssistant = nil;
-
     if (!_textSelectionAssistant)
         _textSelectionAssistant = adoptNS([[UIWKTextInteractionAssistant alloc] initWithView:self]);
     else {
-        // Reset the gesture recognizers in case editibility has changed.
+        // Reset the gesture recognizers in case editability has changed.
         [_textSelectionAssistant setGestureRecognizers];
     }
 }
@@ -4121,7 +4119,7 @@ static NSString *contentTypeFromFieldName(WebCore::AutofillFieldName fieldName)
 
 - (void)_startAssistingKeyboard
 {
-    [self useSelectionAssistantWithGranularity:WKSelectionGranularityCharacter];
+    [self setUpTextSelectionAssistant];
     
     if (self.isFirstResponder && !self.suppressAssistantSelectionView)
         [_textSelectionAssistant activateSelection];
@@ -4133,7 +4131,8 @@ static NSString *contentTypeFromFieldName(WebCore::AutofillFieldName fieldName)
 
 - (void)_stopAssistingKeyboard
 {
-    [self useSelectionAssistantWithGranularity:_webView._selectionGranularity];
+    self.inputDelegate = nil;
+    [self setUpTextSelectionAssistant];
     
     [_textSelectionAssistant deactivateSelection];
 }
index 43f8a4e..9f264f1 100644 (file)
@@ -1,3 +1,32 @@
+2018-10-15  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Can't select text after dismissing the keyboard when changing focus
+        https://bugs.webkit.org/show_bug.cgi?id=190563
+        <rdar://problem/44613559>
+
+        Reviewed by Tim Horton.
+
+        * DumpRenderTree/ios/UIScriptControllerIOS.mm:
+        (WTR::UIScriptController::isShowingKeyboard const):
+
+        Add a new UIScriptController method that returns whether the keyboard is shown. See `ui-helper.js` for more
+        details.
+
+        * TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
+        * TestRunnerShared/UIScriptContext/UIScriptController.cpp:
+        (WTR::UIScriptController::isShowingKeyboard const):
+        * TestRunnerShared/UIScriptContext/UIScriptController.h:
+        * WebKitTestRunner/cocoa/TestRunnerWKWebView.h:
+        * WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:
+
+        Also rename the `isShowingKeyboard` Objective-C property to the more canonical `showingKeyboard`, with
+        `isShowingKeyboard` as the getter method.
+
+        (-[TestRunnerWKWebView _invokeShowKeyboardCallbackIfNecessary]):
+        (-[TestRunnerWKWebView _invokeHideKeyboardCallbackIfNecessary]):
+        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
+        (WTR::UIScriptController::isShowingKeyboard const):
+
 2018-10-15  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, fix JSC tests after WebGPUShadingLanguageRI directory was removed in r237115.
index 89e5cb5..864ec4e 100644 (file)
@@ -234,6 +234,11 @@ void UIScriptController::applyAutocorrection(JSStringRef, JSStringRef, JSValueRe
 {
 }
 
+bool UIScriptController::isShowingKeyboard() const
+{
+    return false;
+}
+
 double UIScriptController::minimumZoomScale() const
 {
     return gWebScrollView.minimumZoomScale;
index 32537d9..bc50082 100644 (file)
@@ -210,6 +210,7 @@ interface UIScriptController {
     // These callbacks also work for the form accessory views.
     attribute object didShowKeyboardCallback;
     attribute object didHideKeyboardCallback;
+    readonly attribute boolean isShowingKeyboard;
 
     attribute object willBeginZoomingCallback;
     attribute object didEndZoomingCallback;
index 731b2cb..28f00aa 100644 (file)
@@ -341,6 +341,11 @@ void UIScriptController::applyAutocorrection(JSStringRef, JSStringRef, JSValueRe
 {
 }
 
+bool UIScriptController::isShowingKeyboard() const
+{
+    return false;
+}
+
 double UIScriptController::zoomScale() const
 {
     return 1;
index cfa3323..60d5966 100644 (file)
@@ -141,6 +141,8 @@ public:
     void setDidHideKeyboardCallback(JSValueRef);
     JSValueRef didHideKeyboardCallback() const;
 
+    bool isShowingKeyboard() const;
+
     void setDidEndScrollingCallback(JSValueRef);
     JSValueRef didEndScrollingCallback() const;
 
index b9b8047..f1ff044 100644 (file)
@@ -49,6 +49,7 @@
 
 @property (nonatomic, assign) UIEdgeInsets overrideSafeAreaInsets;
 
+@property (nonatomic, readonly, getter=isShowingKeyboard) BOOL showingKeyboard;
 @property (nonatomic, assign) BOOL usesSafariLikeRotation;
 @property (nonatomic, readonly, getter=isInteractingWithFormControl) BOOL interactingWithFormControl;
 
index 4670de2..d86cb45 100644 (file)
@@ -54,7 +54,7 @@
 
 @property (nonatomic, copy) void (^zoomToScaleCompletionHandler)(void);
 @property (nonatomic, copy) void (^retrieveSpeakSelectionContentCompletionHandler)(void);
-@property (nonatomic) BOOL isShowingKeyboard;
+@property (nonatomic, getter=isShowingKeyboard, setter=setIsShowingKeyboard:) BOOL showingKeyboard;
 
 @end
 
@@ -154,20 +154,20 @@ IGNORE_WARNINGS_END
 
 - (void)_invokeShowKeyboardCallbackIfNecessary
 {
-    if (self.isShowingKeyboard)
+    if (self.showingKeyboard)
         return;
 
-    self.isShowingKeyboard = YES;
+    self.showingKeyboard = YES;
     if (self.didShowKeyboardCallback)
         self.didShowKeyboardCallback();
 }
 
 - (void)_invokeHideKeyboardCallbackIfNecessary
 {
-    if (!self.isShowingKeyboard)
+    if (!self.showingKeyboard)
         return;
 
-    self.isShowingKeyboard = NO;
+    self.showingKeyboard = NO;
     if (self.didHideKeyboardCallback)
         self.didHideKeyboardCallback();
 }
index 3e437fa..41d6333 100644 (file)
@@ -469,6 +469,11 @@ void UIScriptController::keyboardAccessoryBarPrevious()
     [webView keyboardAccessoryBarPrevious];
 }
 
+bool UIScriptController::isShowingKeyboard() const
+{
+    return TestController::singleton().mainWebView()->platformView().showingKeyboard;
+}
+
 void UIScriptController::applyAutocorrection(JSStringRef newString, JSStringRef oldString, JSValueRef callback)
 {
     unsigned callbackID = m_context->prepareForAsyncTask(callback, CallbackTypeNonPersistent);