Bug 199485

Summary: Single selection <select> with <optgroups> shows multiple selected options
Product: WebKit Reporter: Joao <joaope>
Component: FormsAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, cdumez, darin, dino, glads.logs0o, megan_gardner, thorton, timothy, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: Safari 12   
Hardware: iPhone / iPad   
OS: iOS 12   
Attachments:
Description Flags
Multiple selected option on a single-selection <select>
none
Patch none

Joao
Reported 2019-07-03 17:34:49 PDT
Created attachment 373443 [details] Multiple selected option on a single-selection <select> For a <select> using <optgroup>'s and without multiple option, user is able to select multiple items. Reproduced on an iPhone 12.3.1 but I can see the same behaviour in previous versions. How to reproduce: 1. Go to https://developer.mozilla.org/en-US/docs/Web/HTML/Element/optgroup#Example (or any other website with select/optgroup) 2. Select first option 3. Scroll all the way down to the last option 4. Select last option Both options are now selected. The trick to reproduce seems to be scrolling enough to hide the current selection from the screen and then select another option. It seems a visual bug as JavaScript correctly indicates the current selection. Also using JavaScript to un-select all options but the current one doesn't seem to work (as a workaround). Screenshot in attachment.
Attachments
Multiple selected option on a single-selection <select> (101.03 KB, image/png)
2019-07-03 17:34 PDT, Joao 		no flags
Details
Patch (23.21 KB, patch)
2020-05-17 00:32 PDT, Wenson Hsieh 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2019-07-07 17:52:27 PDT
<rdar://problem/52757531>
Wenson Hsieh
Comment 2 2020-05-16 14:39:12 PDT
*** Bug 211987 has been marked as a duplicate of this bug. ***
Wenson Hsieh
Comment 3 2020-05-16 18:01:03 PDT
This happens because -pickerView:row:column:checked: is not invoked for non-visible picker items, so if an item is checked, is scrolled offscreen, and then another item is checked, we will never update the content view’s FocusedElementInformation to reflect the fact that the previously checked item should now be unchecked. This is pretty simple to fix — we just need to change the `item.isSelected = false;` line in -pickerView:row:column:checked: (in WKFormSelectPicker.mm) to uncheck whatever item was previously checked. …writing a test for this is going to require some finagling, though :|
James Please
Comment 4 2020-05-16 19:09:15 PDT
Thanks for taking the time to look into this and offer such a detailed writeup, Wenson! I really appreciate it. Let me know if I can help out in any way. I found the source on the GitHub mirror, but admittedly I'm not too familiar with this programming language so I'm not sure if I can be useful. I'm willing to give it a try, though! I view this as a high priority bug because it's going to be difficult for anyone who wishes to use the platform to justify using native selects+optgroups if they're aware of this bug. Thanks again!
Wenson Hsieh
Comment 5 2020-05-17 00:32:55 PDT
Created attachment 399585 [details] Patch
EWS
Comment 6 2020-05-18 09:23:33 PDT
Committed r261815: <https://trac.webkit.org/changeset/261815> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399585 [details].
Darin Adler
Comment 7 2020-05-18 12:59:41 PDT
Comment on attachment 399585 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399585&action=review > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8763 > + if ([_inputPeripheral isKindOfClass:[WKFormSelectControl self]]) Normally I see people write one of these: [WKFormSelectControl class] WKFormSelectControl.class I didn’t know that [WKFormSelectControl self] worked. I did a search and see 44 uses of +self in WebKit. But 1083 uses of +class.
Wenson Hsieh
Comment 8 2020-05-18 13:02:31 PDT
(In reply to Darin Adler from comment #7) > Comment on attachment 399585 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=399585&action=review > > > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8763 > > + if ([_inputPeripheral isKindOfClass:[WKFormSelectControl self]]) > > Normally I see people write one of these: > > [WKFormSelectControl class] > WKFormSelectControl.class > > I didn’t know that [WKFormSelectControl self] worked. I did a search and see > 44 uses of +self in WebKit. But 1083 uses of +class. Good point. I just did this to match the style of -selectFormAccessoryPickerRow: above, but we should really just standardize on one (i.e. +class) and use it throughout WebKit. I’ll address this up in a followup.
Note You need to log in before you can comment on or make changes to this bug.