Changeset 263693 in webkit

Timestamp:

Jun 29, 2020 2:26:11 PM (4 years ago)

Author:

Wenson Hsieh

Message:

[iOS] editing/selection/ios/select-text-after-changing-focus.html sometimes fails
​https://bugs.webkit.org/show_bug.cgi?id=213745
Work towards <rdar://problem/64808138>

Reviewed by Tim Horton.

Source/WebKit:

This test first taps an input field to focus it, taps a button below the input field to focus the button and
dismiss the keyboard, and finally long presses to select a word below the button. On recent iOS 14 builds, this
test occasionally fails due to recent changes in UIKit that may cause the callout bar appearance callback to
fire after focusing the input field. If this happens, we end up showing the callout bar with a single option to
"Select All" (despite the field being empty), and the subsequent tap that's intended to hit the button instead
hit-tests to this callout bar item. The tests subsequently times out waiting for the keyboard to dismiss, which
never happens because the input field remains focused.

Showing the "Select All" callout bar option in empty text fields is inconsistent with the rest of the platform
(iOS), and appears to have been unintentionally introduced in iOS 12 by <​https://trac.webkit.org/r231726>. While
it's still unknown which exact system changes caused this test to begin timing out, we can at least fix it by
addressing this existing regression in behavior.

• UIProcess/ios/WKContentViewInteraction.mm:

(-[WKContentView canPerformActionForWebView:withSender:]):

Check -hasContent here, instead of just checking if the selection is a caret. Note that we don't need to check
whether the selection is none separately, since -hasContent will always return NO if there is no selection.

Tools:

See WebKit/ChangeLog for more details. If this test happens to be run after another test that has written
something to the pasteboard, it will still fail due to the callout menu showing up with the option to paste.
Mitigate this by clearing the contents of the system pasteboard between tests, such that content copied from
previous tests doesn't change the behavior of subsequent tests.

• WebKitTestRunner/ios/TestControllerIOS.mm:

(WTR::TestController::platformResetStateToConsistentValues):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/WebKitTestRunner/ios/TestControllerIOS.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-06-29  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] editing/selection/ios/select-text-after-changing-focus.html sometimes fails
+        https://bugs.webkit.org/show_bug.cgi?id=213745
+        Work towards <rdar://problem/64808138>
+
+        Reviewed by Tim Horton.
+
+        This test first taps an input field to focus it, taps a button below the input field to focus the button and
+        dismiss the keyboard, and finally long presses to select a word below the button. On recent iOS 14 builds, this
+        test occasionally fails due to recent changes in UIKit that may cause the callout bar appearance callback to
+        fire after focusing the input field. If this happens, we end up showing the callout bar with a single option to
+        "Select All" (despite the field being empty), and the subsequent tap that's intended to hit the button instead
+        hit-tests to this callout bar item. The tests subsequently times out waiting for the keyboard to dismiss, which
+        never happens because the input field remains focused.
+
+        Showing the "Select All" callout bar option in empty text fields is inconsistent with the rest of the platform
+        (iOS), and appears to have been unintentionally introduced in iOS 12 by <https://trac.webkit.org/r231726>. While
+        it's still unknown which exact system changes caused this test to begin timing out, we can at least fix it by
+        addressing this existing regression in behavior.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView canPerformActionForWebView:withSender:]):
+
+        Check `-hasContent` here, instead of just checking if the selection is a caret. Note that we don't need to check
+        whether the selection is none separately, since `-hasContent` will always return `NO` if there is no selection.
+
 2020-06-29  Sam Weinig  <weinig@apple.com>
 

trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

@@ -3448 +3448 @@
     if (action == @selector(select:)) {
         // Disable select in password fields so that you can't see word boundaries.
-        return !editorState.isInPasswordField && [self hasContent] && !editorState.selectionIsNone && !editorState.selectionIsRange;
+        return !editorState.isInPasswordField && !editorState.selectionIsRange && self.hasContent;
     }
 
     if (action == @selector(selectAll:)) {
-        // By platform convention we don't show Select All in the callout menu for a range selection.
-        if ([sender isKindOfClass:UIMenuController.class])
-            return !editorState.selectionIsNone && !editorState.selectionIsRange;
-#if USE(UIKIT_KEYBOARD_ADDITIONS)
+        if ([sender isKindOfClass:UIMenuController.class]) {
+            // By platform convention we don't show Select All in the callout menu for a range selection.
+            return !editorState.selectionIsRange && self.hasContent;
+        }
         return YES;
-#else
-        return !editorState.selectionIsNone && self.hasContent;
-#endif
     }
 

trunk/Tools/ChangeLog

@@ +1 @@
+2020-06-29  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] editing/selection/ios/select-text-after-changing-focus.html sometimes fails
+        https://bugs.webkit.org/show_bug.cgi?id=213745
+        Work towards <rdar://problem/64808138>
+
+        Reviewed by Tim Horton.
+
+        See WebKit/ChangeLog for more details. If this test happens to be run after another test that has written
+        something to the pasteboard, it will still fail due to the callout menu showing up with the option to paste.
+        Mitigate this by clearing the contents of the system pasteboard between tests, such that content copied from
+        previous tests doesn't change the behavior of subsequent tests.
+
+        * WebKitTestRunner/ios/TestControllerIOS.mm:
+        (WTR::TestController::platformResetStateToConsistentValues):
+
 2020-06-29  Fujii Hironori  <Hironori.Fujii@sony.com>
 

trunk/Tools/WebKitTestRunner/ios/TestControllerIOS.mm

@@ -156 +156 @@
     cocoaResetStateToConsistentValues(options);
 
+    [UIPasteboard generalPasteboard].items = @[ ];
     [[UIApplication sharedApplication] _cancelAllTouches];
     [[UIDevice currentDevice] setOrientation:UIDeviceOrientationPortrait animated:NO];