Title: [274866] trunk
- Revision
- 274866
- Author
- [email protected]
- Date
- 2021-03-23 06:58:29 -0700 (Tue, 23 Mar 2021)
Log Message
[iPadOS] Stale checked item when reopening <select>
https://bugs.webkit.org/show_bug.cgi?id=223592
<rdar://problem/75629529>
Reviewed by Wenson Hsieh.
Source/WebKit:
Reopening a select element after changing its value displays the
original value as checked, rather than the current value on iPadOS. This
is incorrect, since the checked value in the context menu should match
the selected value in-page.
The behavior is incorrect on iPadOS, but works correctly on iPhone, since
dismissing input peripherals on iPhone blurs the focused element,
resulting in a new peripheral being created each time. However, on iPad
the same peripheral is reused when interacting with the same element.
Since the presented UIMenu is only created once during the initialization
of the peripheral, the checked item can be stale on iPads. To fix, update
the menu each time the peripheral is presented, rather than each time it
is created.
Test: fast/forms/ios/form-control-refresh/select/no-stale-checked-items-in-select-picker.html
* UIProcess/ios/forms/WKFormSelectPicker.mm:
(-[WKSelectPicker initWithView:]):
Do not create the presented UIMenu during initialization.
(-[WKSelectPicker controlBeginEditing]):
Create the UIMenu when the peripheral is about to be presented. This
ensures the state of the menu is up-to-date.
(-[WKSelectPicker didSelectOptionIndex:]):
Update the underlying data structure for <select> elements, so that a
newly created UIMenu will have the correct state.
(-[WKSelectPicker createMenu]):
Use a separate local variable to avoid modifying a reference. The
incorrect logic here was masked by the fact that the UIMenu was
previously only created once per WKSelectPicker.
(-[WKSelectPicker actionForOptionIndex:]):
Factored logic to get a UIAction from an option index for testing.
(-[WKSelectPicker selectRow:inComponent:extendingSelection:]):
Call accessoryDone to simulate the dismissal of the presented UIMenu
when using this testing method.
(-[WKSelectPicker selectFormAccessoryHasCheckedItemAtRow:]):
Implement this method so that the checked item can be obtained in tests.
LayoutTests:
Added a test that verifies that tapping on a select element, changing
its value, and then tapping on the same element, presents a context
menu with the correct checked item.
* fast/forms/ios/form-control-refresh/select/no-stale-checked-items-in-select-picker-expected.txt: Added.
* fast/forms/ios/form-control-refresh/select/no-stale-checked-items-in-select-picker.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (274865 => 274866)
--- trunk/LayoutTests/ChangeLog 2021-03-23 13:55:33 UTC (rev 274865)
+++ trunk/LayoutTests/ChangeLog 2021-03-23 13:58:29 UTC (rev 274866)
@@ -1,3 +1,18 @@
+2021-03-23 Aditya Keerthi <[email protected]>
+
+ [iPadOS] Stale checked item when reopening <select>
+ https://bugs.webkit.org/show_bug.cgi?id=223592
+ <rdar://problem/75629529>
+
+ Reviewed by Wenson Hsieh.
+
+ Added a test that verifies that tapping on a select element, changing
+ its value, and then tapping on the same element, presents a context
+ menu with the correct checked item.
+
+ * fast/forms/ios/form-control-refresh/select/no-stale-checked-items-in-select-picker-expected.txt: Added.
+ * fast/forms/ios/form-control-refresh/select/no-stale-checked-items-in-select-picker.html: Added.
+
2021-03-23 Frédéric Wang <[email protected]>
Nullptr deref in WebCore::ApplyStyleCommand::applyRelativeFontStyleChange
Added: trunk/LayoutTests/fast/forms/ios/form-control-refresh/select/no-stale-checked-items-in-select-picker-expected.txt (0 => 274866)
--- trunk/LayoutTests/fast/forms/ios/form-control-refresh/select/no-stale-checked-items-in-select-picker-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/forms/ios/form-control-refresh/select/no-stale-checked-items-in-select-picker-expected.txt 2021-03-23 13:58:29 UTC (rev 274866)
@@ -0,0 +1,13 @@
+This test verifies that tapping on a select element, choosing an option, and then tapping on the same element presents a context menu with the correct checked item.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS select.value is "January"
+PASS checkedItem is "January"
+PASS select.value is "April"
+PASS checkedItem is "April"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/forms/ios/form-control-refresh/select/no-stale-checked-items-in-select-picker.html (0 => 274866)
--- trunk/LayoutTests/fast/forms/ios/form-control-refresh/select/no-stale-checked-items-in-select-picker.html (rev 0)
+++ trunk/LayoutTests/fast/forms/ios/form-control-refresh/select/no-stale-checked-items-in-select-picker.html 2021-03-23 13:58:29 UTC (rev 274866)
@@ -0,0 +1,63 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true IOSFormControlRefreshEnabled=true ] -->
+<html>
+ <head>
+ <meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+ <script src=""
+ <script src=""
+ </head>
+<body>
+<select id="select">
+ <option>January</option>
+ <option>February</option>
+ <option>March</option>
+ <option>April</option>
+ <option>May</option>
+ <option>June</option>
+ <option>July</option>
+ <option>August</option>
+ <option>September</option>
+ <option>October</option>
+ <option>November</option>
+ <option>December</option>
+</select>
+</body>
+<script>
+jsTestIsAsync = true;
+
+async function verifyCheckedItemInContextMenu(expectedItem)
+{
+ checkedItem = "";
+ for (const [index, option] of Array.from(select.options).entries()) {
+ let isChecked = await UIHelper.selectFormAccessoryHasCheckedItemAtRow(index);
+ if (isChecked) {
+ if (checkedItem)
+ testFailed("More than one checked item.");
+
+ checkedItem = option.value;
+ }
+ }
+
+ shouldBeEqualToString("checkedItem", expectedItem);
+}
+
+addEventListener("load", async () => {
+ description("This test verifies that tapping on a select element, choosing an option, and then tapping on the same element presents a context menu with the correct checked item.");
+
+ shouldBeEqualToString("select.value", "January");
+ await UIHelper.activateElement(select);
+ await UIHelper.waitForContextMenuToShow();
+ await verifyCheckedItemInContextMenu("January");
+ await UIHelper.selectFormAccessoryPickerRow(3);
+ await UIHelper.waitForContextMenuToHide();
+ shouldBeEqualToString("select.value", "April");
+
+ await UIHelper.activateElement(select);
+ await UIHelper.waitForContextMenuToShow();
+ await verifyCheckedItemInContextMenu("April");
+ await UIHelper.selectFormAccessoryPickerRow(0);
+ await UIHelper.waitForContextMenuToHide();
+
+ finishJSTest();
+});
+</script>
+</html>
Modified: trunk/Source/WebKit/ChangeLog (274865 => 274866)
--- trunk/Source/WebKit/ChangeLog 2021-03-23 13:55:33 UTC (rev 274865)
+++ trunk/Source/WebKit/ChangeLog 2021-03-23 13:58:29 UTC (rev 274866)
@@ -1,3 +1,61 @@
+2021-03-23 Aditya Keerthi <[email protected]>
+
+ [iPadOS] Stale checked item when reopening <select>
+ https://bugs.webkit.org/show_bug.cgi?id=223592
+ <rdar://problem/75629529>
+
+ Reviewed by Wenson Hsieh.
+
+ Reopening a select element after changing its value displays the
+ original value as checked, rather than the current value on iPadOS. This
+ is incorrect, since the checked value in the context menu should match
+ the selected value in-page.
+
+ The behavior is incorrect on iPadOS, but works correctly on iPhone, since
+ dismissing input peripherals on iPhone blurs the focused element,
+ resulting in a new peripheral being created each time. However, on iPad
+ the same peripheral is reused when interacting with the same element.
+ Since the presented UIMenu is only created once during the initialization
+ of the peripheral, the checked item can be stale on iPads. To fix, update
+ the menu each time the peripheral is presented, rather than each time it
+ is created.
+
+ Test: fast/forms/ios/form-control-refresh/select/no-stale-checked-items-in-select-picker.html
+
+ * UIProcess/ios/forms/WKFormSelectPicker.mm:
+ (-[WKSelectPicker initWithView:]):
+
+ Do not create the presented UIMenu during initialization.
+
+ (-[WKSelectPicker controlBeginEditing]):
+
+ Create the UIMenu when the peripheral is about to be presented. This
+ ensures the state of the menu is up-to-date.
+
+ (-[WKSelectPicker didSelectOptionIndex:]):
+
+ Update the underlying data structure for <select> elements, so that a
+ newly created UIMenu will have the correct state.
+
+ (-[WKSelectPicker createMenu]):
+
+ Use a separate local variable to avoid modifying a reference. The
+ incorrect logic here was masked by the fact that the UIMenu was
+ previously only created once per WKSelectPicker.
+
+ (-[WKSelectPicker actionForOptionIndex:]):
+
+ Factored logic to get a UIAction from an option index for testing.
+
+ (-[WKSelectPicker selectRow:inComponent:extendingSelection:]):
+
+ Call accessoryDone to simulate the dismissal of the presented UIMenu
+ when using this testing method.
+
+ (-[WKSelectPicker selectFormAccessoryHasCheckedItemAtRow:]):
+
+ Implement this method so that the checked item can be obtained in tests.
+
2021-03-23 Kimmo Kinnunen <[email protected]>
Move instanced drawing functionality from ExtensionsGL to GraphicsContextGL
Modified: trunk/Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm (274865 => 274866)
--- trunk/Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm 2021-03-23 13:55:33 UTC (rev 274865)
+++ trunk/Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm 2021-03-23 13:58:29 UTC (rev 274866)
@@ -490,9 +490,6 @@
_view = view;
_interactionPoint = [_view lastInteractionLocation];
-#if USE(UICONTEXTMENU)
- _selectMenu = [self createMenu];
-#endif
return self;
}
@@ -507,6 +504,8 @@
[_view startRelinquishingFirstResponderToFocusedElement];
#if USE(UICONTEXTMENU)
+ _selectMenu = [self createMenu];
+
WebKit::InteractionInformationRequest positionInformationRequest { WebCore::IntPoint(_view.focusedElementInformation.lastInteractionLocation) };
[_view doAfterPositionInformationUpdate:^(WebKit::InteractionInformationAtPosition interactionInformation) {
[self showSelectPicker];
@@ -533,6 +532,15 @@
- (void)didSelectOptionIndex:(NSInteger)index
{
+ NSInteger optionIndex = 0;
+ for (auto& option : _view.focusedSelectElementOptions) {
+ if (option.isGroup)
+ continue;
+
+ option.isSelected = optionIndex == index;
+ optionIndex++;
+ }
+
[_view page]->setFocusedElementSelectedIndex(index);
}
@@ -558,11 +566,11 @@
currentIndex++;
while (currentIndex < _view.focusedSelectElementOptions.size()) {
- optionItem = _view.focusedSelectElementOptions[currentIndex];
- if (optionItem.isGroup)
+ auto& childOptionItem = _view.focusedSelectElementOptions[currentIndex];
+ if (childOptionItem.isGroup)
break;
- UIAction *action = "" actionForOptionItem:optionItem withIndex:optionIndex];
+ UIAction *action = "" actionForOptionItem:childOptionItem withIndex:optionIndex];
[groupedItems addObject:action];
optionIndex++;
currentIndex++;
@@ -597,6 +605,30 @@
return optionAction;
}
+- (UIAction *)actionForOptionIndex:(NSInteger)optionIndex
+{
+ NSInteger currentIndex = 0;
+
+ NSArray<UIMenuElement *> *menuElements = [_selectMenu children];
+ for (UIMenuElement *menuElement in menuElements) {
+ if ([menuElement isKindOfClass:UIAction.class]) {
+ if (currentIndex == optionIndex)
+ return (UIAction *)menuElement;
+
+ currentIndex++;
+ continue;
+ }
+
+ UIMenu *groupedMenu = (UIMenu *)menuElement;
+ if (currentIndex + groupedMenu.children.count <= (NSUInteger)optionIndex)
+ currentIndex += groupedMenu.children.count;
+ else
+ return (UIAction *)[groupedMenu.children objectAtIndex:optionIndex - currentIndex];
+ }
+
+ return nil;
+}
+
- (UITargetedPreview *)contextMenuInteraction:(UIContextMenuInteraction *)interaction previewForHighlightingMenuWithConfiguration:(UIContextMenuConfiguration *)configuration
{
return [_view _createTargetedContextMenuHintPreviewIfPossible];
@@ -671,35 +703,27 @@
#endif // USE(UICONTEXTMENU)
// WKSelectTesting
+
- (void)selectRow:(NSInteger)rowIndex inComponent:(NSInteger)componentIndex extendingSelection:(BOOL)extendingSelection
{
#if USE(UICONTEXTMENU)
- NSInteger currentRow = 0;
-
- NSArray<UIMenuElement *> *menuElements = [_selectMenu children];
- for (UIMenuElement *menuElement in menuElements) {
- if ([menuElement isKindOfClass:UIAction.class]) {
- if (currentRow == rowIndex) {
- [(UIAction *)menuElement _performActionWithSender:nil];
- break;
- }
-
- currentRow++;
- continue;
- }
-
- UIMenu *groupedMenu = (UIMenu *)menuElement;
- if (currentRow + groupedMenu.children.count <= (NSUInteger)rowIndex)
- currentRow += groupedMenu.children.count;
- else {
- UIAction *action = "" *)[groupedMenu.children objectAtIndex:rowIndex - currentRow];
- [action _performActionWithSender:nil];
- break;
- }
+ UIAction *optionAction = [self actionForOptionIndex:rowIndex];
+ if (optionAction) {
+ [optionAction _performActionWithSender:nil];
+ [_view accessoryDone];
}
+#endif
+}
- [self removeContextMenuInteraction];
+- (BOOL)selectFormAccessoryHasCheckedItemAtRow:(long)rowIndex
+{
+#if USE(UICONTEXTMENU)
+ UIAction *optionAction = [self actionForOptionIndex:rowIndex];
+ if (optionAction)
+ return optionAction.state == UIMenuElementStateOn;
#endif
+
+ return NO;
}
@end
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
