Title: [287381] trunk
Revision
287381
Author
[email protected]
Date
2021-12-22 18:19:55 -0800 (Wed, 22 Dec 2021)

Log Message

[iOS] metromile.com <select> dropdowns open twice
https://bugs.webkit.org/show_bug.cgi?id=234573
rdar://84011144

Reviewed by Wenson Hsieh.

Source/WebKit:

metromile.com calls focus() twice on the same <select> element, when one
is tapped. Note that calling focus() on an already focused element still
sends the ElementDidFocus message to the UIProcess, to support scenarios
where the initial call to focus did not bring up an input peripheral.
One example of such a scenario is when the initial call is not a result
of user interaction, however, there are additional scenarios that are
dependent on UIProcess state. This makes it difficult to ensure that
ElementDidFocus is only called when it is time to show an input peripheral.

`-[WKContentView _elementDidFocus:...]` already contains some logic to
prevent showing another input peripheral for the same element. This
logic was added in r168744, checking for the same element by comparing
input types and bounding rects.

However, in between the first and second call to focus(), the page
applies additional padding to the element, changing its bounding rect.
This results in `_elementDidFocus:` going past the same element check
and presenting another dropdown.

In the time since r168744 was written, a more robust mechanism to check
for the same element was added – ElementContext.

To fix, update the same element check to use ElementContext, ensuring an
attempt to present a new peripheral for the same element is not made.

Test: fast/forms/ios/refocus-select-after-size-change.html

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:activityStateChanges:userObject:]):

LayoutTests:

Added a layout test to verify that only one <select> dropdown is
presented when refocusing the same <select> element with a different
size. If more than one dropdown is presented, the test times out.

* fast/forms/ios/refocus-select-after-size-change-expected.txt: Added.
* fast/forms/ios/refocus-select-after-size-change.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (287380 => 287381)


--- trunk/LayoutTests/ChangeLog	2021-12-23 01:48:18 UTC (rev 287380)
+++ trunk/LayoutTests/ChangeLog	2021-12-23 02:19:55 UTC (rev 287381)
@@ -1,3 +1,18 @@
+2021-12-22  Aditya Keerthi  <[email protected]>
+
+        [iOS] metromile.com <select> dropdowns open twice
+        https://bugs.webkit.org/show_bug.cgi?id=234573
+        rdar://84011144
+
+        Reviewed by Wenson Hsieh.
+
+        Added a layout test to verify that only one <select> dropdown is
+        presented when refocusing the same <select> element with a different
+        size. If more than one dropdown is presented, the test times out.
+
+        * fast/forms/ios/refocus-select-after-size-change-expected.txt: Added.
+        * fast/forms/ios/refocus-select-after-size-change.html: Added.
+
 2021-12-22  Simon Fraser  <[email protected]>
 
         Convert css3/scroll-snap/scroll-snap-wheel-event.html to use monitorWheelEvents()

Added: trunk/LayoutTests/fast/forms/ios/refocus-select-after-size-change-expected.txt (0 => 287381)


--- trunk/LayoutTests/fast/forms/ios/refocus-select-after-size-change-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/ios/refocus-select-after-size-change-expected.txt	2021-12-23 02:19:55 UTC (rev 287381)
@@ -0,0 +1,11 @@
+This test verifies that refocusing a select element after changing its size does not present two pickers.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+

Added: trunk/LayoutTests/fast/forms/ios/refocus-select-after-size-change.html (0 => 287381)


--- trunk/LayoutTests/fast/forms/ios/refocus-select-after-size-change.html	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/ios/refocus-select-after-size-change.html	2021-12-23 02:19:55 UTC (rev 287381)
@@ -0,0 +1,53 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+    <script src=""
+    <script src=""
+    <style>
+        select {
+            width: 100px;
+            height: 100px;
+        }
+
+        .focused {
+            width: 101px;
+            height: 101px;
+        }
+    </style>
+</head>
+<body>
+    <input type="button" id="showSelectButton" value="Show select">
+    <input type="button" id="finishTestButton" value="Finish test">
+    <br>
+    <select id="select">
+        <option value="A">A</option>
+        <option value="B">B</option>
+        <option value="C">C</option>
+    </select>
+</body>
+<script>
+jsTestIsAsync = true;
+
+addEventListener("load", async () => {
+    description("This test verifies that refocusing a select element after changing its size does not present two pickers.");
+
+    finishTestButton.addEventListener("click", () => {
+        testPassed("");
+        finishJSTest();
+    });
+
+    showSelectButton.addEventListener("click", async () => {
+        select.focus();
+        select.classList.add("focused");
+        select.focus();
+
+        await UIHelper.ensurePresentationUpdate();
+        await UIHelper.selectFormAccessoryPickerRow(0);
+        await UIHelper.activateElement(finishTestButton);
+    });
+
+    UIHelper.activateElement(showSelectButton);
+});
+</script>
+</html>

Modified: trunk/Source/WebKit/ChangeLog (287380 => 287381)


--- trunk/Source/WebKit/ChangeLog	2021-12-23 01:48:18 UTC (rev 287380)
+++ trunk/Source/WebKit/ChangeLog	2021-12-23 02:19:55 UTC (rev 287381)
@@ -1,3 +1,41 @@
+2021-12-22  Aditya Keerthi  <[email protected]>
+
+        [iOS] metromile.com <select> dropdowns open twice
+        https://bugs.webkit.org/show_bug.cgi?id=234573
+        rdar://84011144
+
+        Reviewed by Wenson Hsieh.
+
+        metromile.com calls focus() twice on the same <select> element, when one
+        is tapped. Note that calling focus() on an already focused element still
+        sends the ElementDidFocus message to the UIProcess, to support scenarios
+        where the initial call to focus did not bring up an input peripheral.
+        One example of such a scenario is when the initial call is not a result
+        of user interaction, however, there are additional scenarios that are
+        dependent on UIProcess state. This makes it difficult to ensure that
+        ElementDidFocus is only called when it is time to show an input peripheral.
+
+        `-[WKContentView _elementDidFocus:...]` already contains some logic to
+        prevent showing another input peripheral for the same element. This
+        logic was added in r168744, checking for the same element by comparing
+        input types and bounding rects.
+
+        However, in between the first and second call to focus(), the page
+        applies additional padding to the element, changing its bounding rect.
+        This results in `_elementDidFocus:` going past the same element check
+        and presenting another dropdown.
+
+        In the time since r168744 was written, a more robust mechanism to check
+        for the same element was added – ElementContext.
+
+        To fix, update the same element check to use ElementContext, ensuring an
+        attempt to present a new peripheral for the same element is not made.
+
+        Test: fast/forms/ios/refocus-select-after-size-change.html
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:activityStateChanges:userObject:]):
+
 2021-12-22  Sihui Liu  <[email protected]>
 
         WebsiteDataStore::excludeDirectoryFromBackup should set attribute for existing directories

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (287380 => 287381)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2021-12-23 01:48:18 UTC (rev 287380)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2021-12-23 02:19:55 UTC (rev 287381)
@@ -6457,7 +6457,7 @@
 
     // FIXME: We should remove this check when we manage to send ElementDidFocus from the WebProcess
     // only when it is truly time to show the keyboard.
-    if (_focusedElementInformation.elementType == information.elementType && _focusedElementInformation.interactionRect == information.interactionRect) {
+    if (self._hasFocusedElement && _focusedElementInformation.elementContext.isSameElement(information.elementContext)) {
         if (_inputPeripheral) {
             if (!self.isFirstResponder)
                 [self becomeFirstResponder];
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to