Title: [266194] trunk
Revision
266194
Author
[email protected]
Date
2020-08-26 15:03:39 -0700 (Wed, 26 Aug 2020)

Log Message

[iOS] Disabled options in the multi-select picker should not be selectable
https://bugs.webkit.org/show_bug.cgi?id=201458
<rdar://problem/55018179>

Reviewed by Wenson Hsieh.

Source/WebKit:

WKMultipleSelectPicker is displayed when a <select multiple> or a
<select> with an <optgroup> is activated. Due to <rdar://problem/18745253>,
group rows and disabled rows were selectable in the picker. r175266 added a
workaround for this issue, preventing group rows from being selectable.
However, the workaround did not account for disabled rows, which means that
it is possible to select disabled options in the multi-select picker.

To fix this behavior, the same fix that was applied to group rows is now
applied to disabled rows. `pickerView:row:column:checked:` resets the style
for the associated view if `item.disabled` is true.

Test: fast/forms/ios/disabled-options-in-multi-select-picker.html

* UIProcess/ios/forms/WKFormSelectPicker.mm:
(-[WKMultipleSelectPicker pickerView:row:column:checked:]):

LayoutTests:

Added a test to verify that disabled options cannot be selected when the
multi-select picker is presented.

* fast/forms/ios/disabled-options-in-multi-select-picker-expected.txt: Added.
* fast/forms/ios/disabled-options-in-multi-select-picker.html: Added.
* platform/ipad/TestExpectations:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (266193 => 266194)


--- trunk/LayoutTests/ChangeLog	2020-08-26 21:32:50 UTC (rev 266193)
+++ trunk/LayoutTests/ChangeLog	2020-08-26 22:03:39 UTC (rev 266194)
@@ -1,3 +1,18 @@
+2020-08-26  Aditya Keerthi  <[email protected]>
+
+        [iOS] Disabled options in the multi-select picker should not be selectable
+        https://bugs.webkit.org/show_bug.cgi?id=201458
+        <rdar://problem/55018179>
+
+        Reviewed by Wenson Hsieh.
+
+        Added a test to verify that disabled options cannot be selected when the
+        multi-select picker is presented.
+
+        * fast/forms/ios/disabled-options-in-multi-select-picker-expected.txt: Added.
+        * fast/forms/ios/disabled-options-in-multi-select-picker.html: Added.
+        * platform/ipad/TestExpectations:
+
 2020-08-26  Hector Lopez  <[email protected]>
 
         rdar://67706887 (REGRESSION (r264950): [ iOS 13 WK2 ] imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml.html is a constant failure)

Added: trunk/LayoutTests/fast/forms/ios/disabled-options-in-multi-select-picker-expected.txt (0 => 266194)


--- trunk/LayoutTests/fast/forms/ios/disabled-options-in-multi-select-picker-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/ios/disabled-options-in-multi-select-picker-expected.txt	2020-08-26 22:03:39 UTC (rev 266194)
@@ -0,0 +1,28 @@
+This test verifies that disabled options cannot be selected in the multi-select picker on iPhones.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Test select multiple
+
+PASS selected is true
+PASS selected is false
+PASS selected is true
+PASS selected is false
+
+Test select with optgroup
+
+PASS selected is true
+PASS selectElement.value is "1"
+PASS selected is false
+PASS selectElement.value is "1"
+PASS selected is false
+PASS selectElement.value is "1"
+PASS selected is true
+PASS selectElement.value is "3"
+PASS selected is false
+PASS selectElement.value is "3"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+  

Added: trunk/LayoutTests/fast/forms/ios/disabled-options-in-multi-select-picker.html (0 => 266194)


--- trunk/LayoutTests/fast/forms/ios/disabled-options-in-multi-select-picker.html	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/ios/disabled-options-in-multi-select-picker.html	2020-08-26 22:03:39 UTC (rev 266194)
@@ -0,0 +1,82 @@
+<!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;
+        }
+    </style>
+</head>
+<body>
+    <select id="select-multiple" multiple>
+        <option value="1">One</option>
+        <option value="2" disabled>Two</option>
+        <option value="3">Three</option>
+        <option value="4" disabled>Four</option>
+    </select>
+
+    <select id="select-optgroup">
+        <option value="1">One</option>
+        <option value="2" disabled>Two</option>
+        <optgroup label="Group">
+            <option value="3">Three</option>
+            <option value="4" disabled>Four</option>
+        </optgroup>
+    </select>
+</body>
+<script>
+jsTestIsAsync = true;
+
+async function attemptSelectForElementAndRow(element, row) {
+    selectElement = element;
+    option = element.getElementsByTagName("*")[row];
+    previousValue = element.value;
+
+    await UIHelper.selectFormAccessoryPickerRow(row);
+    selected = await UIHelper.selectFormAccessoryHasCheckedItemAtRow(row);
+
+    if (option instanceof HTMLOptGroupElement || option.disabled) {
+        expectedValue = previousValue;
+        shouldBeFalse("selected");
+    } else {
+        expectedValue = option.value;
+        shouldBeTrue("selected");
+    }
+
+    // <select multiple> does not update the value until the accessory is
+    // dismissed. In that case, the previous assertion on `selected` is
+    // enough for this test.
+    if (!selectElement.multiple)
+        shouldBeEqualToString("selectElement.value", expectedValue);
+}
+
+async function testAllItemsInSelectElement(element) {
+    await UIHelper.activateElementAndWaitForInputSession(element);
+
+    for (i = 0; i < element.getElementsByTagName("*").length; i++)
+        await attemptSelectForElementAndRow(element, i);
+
+    element.blur();
+    await UIHelper.waitForKeyboardToHide();
+}
+
+addEventListener("load", async () => {
+    description("This test verifies that disabled options cannot be selected in the multi-select picker on iPhones.");
+
+    debug("Test select multiple\n");
+    const selectMultiple = document.getElementById("select-multiple");
+    await testAllItemsInSelectElement(selectMultiple);
+
+    // <select> with an <optgroup> displays the multi-select picker on iPhones.
+    debug("\nTest select with optgroup\n");
+    const selectWithGroup = document.getElementById("select-optgroup");
+    await testAllItemsInSelectElement(selectWithGroup);
+
+    finishJSTest();
+});
+</script>
+</html>

Modified: trunk/LayoutTests/platform/ipad/TestExpectations (266193 => 266194)


--- trunk/LayoutTests/platform/ipad/TestExpectations	2020-08-26 21:32:50 UTC (rev 266193)
+++ trunk/LayoutTests/platform/ipad/TestExpectations	2020-08-26 22:03:39 UTC (rev 266194)
@@ -10,6 +10,7 @@
 
 # The select picker input view is not displayed on iPad.
 fast/forms/ios/no-stale-checked-items-in-select-picker.html
+fast/forms/ios/disabled-options-in-multi-select-picker.html
 
 # These tests are designed for iPhone and crash on iPad
 media/modern-media-controls/media-documents/media-document-audio-ios-sizing.html [ Skip ]

Modified: trunk/Source/WebKit/ChangeLog (266193 => 266194)


--- trunk/Source/WebKit/ChangeLog	2020-08-26 21:32:50 UTC (rev 266193)
+++ trunk/Source/WebKit/ChangeLog	2020-08-26 22:03:39 UTC (rev 266194)
@@ -1,3 +1,27 @@
+2020-08-26  Aditya Keerthi  <[email protected]>
+
+        [iOS] Disabled options in the multi-select picker should not be selectable
+        https://bugs.webkit.org/show_bug.cgi?id=201458
+        <rdar://problem/55018179>
+
+        Reviewed by Wenson Hsieh.
+
+        WKMultipleSelectPicker is displayed when a <select multiple> or a
+        <select> with an <optgroup> is activated. Due to <rdar://problem/18745253>,
+        group rows and disabled rows were selectable in the picker. r175266 added a
+        workaround for this issue, preventing group rows from being selectable.
+        However, the workaround did not account for disabled rows, which means that
+        it is possible to select disabled options in the multi-select picker.
+
+        To fix this behavior, the same fix that was applied to group rows is now
+        applied to disabled rows. `pickerView:row:column:checked:` resets the style
+        for the associated view if `item.disabled` is true.
+
+        Test: fast/forms/ios/disabled-options-in-multi-select-picker.html
+
+        * UIProcess/ios/forms/WKFormSelectPicker.mm:
+        (-[WKMultipleSelectPicker pickerView:row:column:checked:]):
+
 2020-08-26  Youenn Fablet  <[email protected]>
 
         Enable WritableStream by default

Modified: trunk/Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm (266193 => 266194)


--- trunk/Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm	2020-08-26 21:32:50 UTC (rev 266193)
+++ trunk/Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm	2020-08-26 22:03:39 UTC (rev 266194)
@@ -279,13 +279,14 @@
     auto& item = [_view focusedSelectElementOptions][rowIndex];
 
     // FIXME: Remove this workaround once <rdar://problem/18745253> is fixed.
-    // Group rows should not be checkable, but we are getting this delegate for
-    // those rows. As a workaround, if we get this delegate for a group row, reset
-    // the styles for the content view so it still appears unselected.
-    if (item.isGroup) {
+    // Group rows and disabled rows should not be checkable, but we are getting
+    // this delegate for those rows. As a workaround, if we get this delegate
+    // for a group or disabled row, reset the styles for the content view so it
+    // still appears unselected.
+    if (item.isGroup || item.disabled) {
         UIPickerContentView *view = (UIPickerContentView *)[self viewForRow:rowIndex forComponent:columnIndex];
         [view setChecked:NO];
-        [[view titleLabel] setTextColor:[UIColor colorWithWhite:0.0 alpha:GroupOptionTextColorAlpha]];
+        [[view titleLabel] setTextColor:[UIColor colorWithWhite:0.0 alpha:item.isGroup ? GroupOptionTextColorAlpha : DisabledOptionAlpha]];
         return;
     }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to