Changeset 265117 in webkit

Timestamp:

Jul 30, 2020, 4:31:59 PM (6 years ago)

Author:

Wenson Hsieh

Message:

[iPadOS] Two taps are required to choose a country/region when creating an account on ea.com
​https://bugs.webkit.org/show_bug.cgi?id=214984
<rdar://problem/66261490>

Reviewed by Devin Rousso.

Source/WebKit:

On the desktop version of the account creation page on ea.com, tapping a custom selection menu element
programmatically focuses a real select element that is positioned far outside the viewport. As a result, on
iPad, we still attempt to show a popover for this focused select at the element location. This popover is not
visible to the user, but on iOS 13, it still steals modal focus from the web view, which means that an
additional tap anywhere in the window is required to dismiss the (invisible) popover before the user can choose
one of the options in the custom in-page menu that is shown.

Furthermore, upon choosing one of these options, the page programmatically focuses the offscreen select again.
This causes us to re-present the offscreen popover, which requires yet another tap to dismiss on iOS 13.
However, on iOS 14, this actually results in a crash when attempting to scroll the popover's inner UITableView
to the selected row.

To fix both the crash on iOS 14 and the multiple redundant taps required to dismiss the popover on iOS 13, avoid
showing popovers altogether if the popover's presentation rect is outside the bounds of the content view, since
we cannot reasonably expect a user to interact with such a popover anyways.

Test: fast/forms/ios/repeatedly-focus-offscreen-select.html

• UIProcess/ios/forms/WKFormPopover.mm:

(-[WKRotatingPopover presentPopoverAnimated:]):

LayoutTests:

Add a test that exercises both the crash and the UI bug on iPadOS. Note that this test already passes on iOS,
where we don't use popovers for focused selects.

• fast/forms/ios/repeatedly-focus-offscreen-select-expected.txt: Added.
• fast/forms/ios/repeatedly-focus-offscreen-select.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/forms/ios/repeatedly-focus-offscreen-select-expected.txt (added)
• LayoutTests/fast/forms/ios/repeatedly-focus-offscreen-select.html (added)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/ios/forms/WKFormPopover.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-07-30  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iPadOS] Two taps are required to choose a country/region when creating an account on ea.com
+        https://bugs.webkit.org/show_bug.cgi?id=214984
+        <rdar://problem/66261490>
+
+        Reviewed by Devin Rousso.
+
+        Add a test that exercises both the crash and the UI bug on iPadOS. Note that this test already passes on iOS,
+        where we don't use popovers for focused selects.
+
+        * fast/forms/ios/repeatedly-focus-offscreen-select-expected.txt: Added.
+        * fast/forms/ios/repeatedly-focus-offscreen-select.html: Added.
+
 2020-07-30  Antoine Quint  <graouts@webkit.org>
 

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-07-30  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iPadOS] Two taps are required to choose a country/region when creating an account on ea.com
+        https://bugs.webkit.org/show_bug.cgi?id=214984
+        <rdar://problem/66261490>
+
+        Reviewed by Devin Rousso.
+
+        On the desktop version of the account creation page on ea.com, tapping a custom selection menu element
+        programmatically focuses a real `select` element that is positioned far outside the viewport. As a result, on
+        iPad, we still attempt to show a popover for this focused select at the element location. This popover is not
+        visible to the user, but on iOS 13, it still steals modal focus from the web view, which means that an
+        additional tap anywhere in the window is required to dismiss the (invisible) popover before the user can choose
+        one of the options in the custom in-page menu that is shown.
+
+        Furthermore, upon choosing one of these options, the page programmatically focuses the offscreen select again.
+        This causes us to re-present the offscreen popover, which requires yet another tap to dismiss on iOS 13.
+        However, on iOS 14, this actually results in a crash when attempting to scroll the popover's inner UITableView
+        to the selected row.
+
+        To fix both the crash on iOS 14 and the multiple redundant taps required to dismiss the popover on iOS 13, avoid
+        showing popovers altogether if the popover's presentation rect is outside the bounds of the content view, since
+        we cannot reasonably expect a user to interact with such a popover anyways.
+
+        Test: fast/forms/ios/repeatedly-focus-offscreen-select.html
+
+        * UIProcess/ios/forms/WKFormPopover.mm:
+        (-[WKRotatingPopover presentPopoverAnimated:]):
+
 2020-07-30  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebKit/UIProcess/ios/forms/WKFormPopover.mm

@@ -134 +134 @@
 - (void)presentPopoverAnimated:(BOOL)animated
 {
+    auto directions = [self popoverArrowDirections];
+    CGRect presentationRect;
+    if (CGPointEqualToPoint(self.presentationPoint, CGPointZero))
+        presentationRect = _view.focusedElementInformation.interactionRect;
+    else {
+        auto scale = _view.page->pageScaleFactor();
+        presentationRect = CGRectMake(self.presentationPoint.x * scale, self.presentationPoint.y * scale, 1, 1);
+    }
+
+    if (!CGRectIntersectsRect(presentationRect, _view.bounds))
+        return;
+
 #if PLATFORM(MACCATALYST)
     [_view startRelinquishingFirstResponderToFocusedElement];
 #endif
-
-    UIPopoverArrowDirection directions = [self popoverArrowDirections];
-
-    BOOL presentWithPoint = !CGPointEqualToPoint(self.presentationPoint, CGPointZero);
-    if (presentWithPoint) {
-        CGFloat scale = [_view page]->pageScaleFactor();
-        [_popoverController presentPopoverFromRect:CGRectIntegral(CGRectMake(self.presentationPoint.x * scale, self.presentationPoint.y * scale, 1, 1))
-                                            inView:_view
-                          permittedArrowDirections:directions
-                                          animated:animated];
-    } else {
-        CGRect boundingBoxOfDOMNode = _view.focusedElementInformation.interactionRect;
-        [_popoverController presentPopoverFromRect:CGRectIntegral(boundingBoxOfDOMNode)
-                                            inView:_view
-                          permittedArrowDirections:directions
-                                          animated:animated];
-    }
+    [_popoverController presentPopoverFromRect:CGRectIntegral(presentationRect) inView:_view permittedArrowDirections:directions animated:animated];
 }