-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consume user activation in show the picker #10344
Consume user activation in show the picker #10344
Conversation
More in-depth alternative to whatwg#10098. Fixes whatwg#10084, helps GHSA-hr74-5fj7-jgxp.
This seems to accurately describe what I implemented in chromium here, so I am supportive: https://chromium-review.googlesource.com/c/chromium/src/+/5235516 |
@josepharhar one key difference is that this applies to the main algorithm not just the showPicker() function. So there's a difference here from your patch. |
Ah ok, I can see that calling element.click() on I think it would probably be a good idea to make that require user activation though since it opens a popup document. I can guard the change behind a flag and revert it if websites are relying too much on it or something. Let's do it! The WPT should probably test the element.click() cases, right? This algorithm does get called when doing element.click(), right? |
Yes and yes |
My only concern with this is, stylable select will be a popover so does it really need to be concerned with user activation? Is it possible we should scope this to just pickers that can escape the bounds (current select and file?). Either way I think that's a concern we can address in future, just wanted to point it out. |
No it shouldn't, and you can just call showPopover() on the datalist element. However, stylable select doesn't exist in the spec yet (hopefully it will someday), and if that really is an issue, then we can patch the algorithm to check whether it's in appearance:base-select mode or not.
My only concern is still that we would break existing websites, so I would launch the change behind a flag and disable+revert it if we get a lot of feedback about it. |
For consistency and safety we should apply this to everything. For example in Firefox the |
Spec PR: whatwg/html#10344. Depends/based on web-platform-tests#46500.
WPT PR: web-platform-tests/wpt#46502 |
What if there's no |
I suspect we probably want this documented on MDN in due course so please file an issue. Also seems good to file bugs against Chromium and WebKit for this. |
Filed bugs for vendors and MDN. Are there any other blockers for this merging at the moment? |
Thanks @CanadaHonk for fixing this! |
Allowing the page to call showPicker() on select elements as much as it wants without consuming user activation may result in the user being unable to interact with the browser UI due to popups always taking focus. The HTML spec also says to do this for input elements, so I added code to do it there as well. This patch also modified the select showPicker test because calling showPicker on a select twice in a row in the test somehow resulted in blink not seeing any input events on the second test_driver.bless(), perhaps because the select's popup is still open and is somehow intercepting the input. HTML spec: whatwg/html#10344 Bug: 1521345 Fixed: 343302069, 343093082, 343473478 Change-Id: If6308a67bac9050f695d18d275ea86c23ac22b0d
Allowing the page to call showPicker() on select elements as much as it wants without consuming user activation may result in the user being unable to interact with the browser UI due to popups always taking focus. The HTML spec also says to do this for input elements, so I added code to do it there as well. This patch also modified the select showPicker test because calling showPicker on a select twice in a row in the test somehow resulted in blink not seeing any input events on the second test_driver.bless(), perhaps because the select's popup is still open and is somehow intercepting the input. HTML spec: whatwg/html#10344 Bug: 1521345 Fixed: 343302069, 343093082, 343473478 Change-Id: If6308a67bac9050f695d18d275ea86c23ac22b0d
Allowing the page to call showPicker() on select elements as much as it wants without consuming user activation may result in the user being unable to interact with the browser UI due to popups always taking focus. The HTML spec also says to do this for input elements, so I added code to do it there as well. This patch also modified the select showPicker test because calling showPicker on a select twice in a row in the test somehow resulted in blink not seeing any input events on the second test_driver.bless(), perhaps because the select's popup is still open and is somehow intercepting the input. HTML spec: whatwg/html#10344 Bug: 1521345 Fixed: 343302069, 343093082, 343473478 Change-Id: If6308a67bac9050f695d18d275ea86c23ac22b0d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5235516 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1309009}
Allowing the page to call showPicker() on select elements as much as it wants without consuming user activation may result in the user being unable to interact with the browser UI due to popups always taking focus. The HTML spec also says to do this for input elements, so I added code to do it there as well. This patch also modified the select showPicker test because calling showPicker on a select twice in a row in the test somehow resulted in blink not seeing any input events on the second test_driver.bless(), perhaps because the select's popup is still open and is somehow intercepting the input. HTML spec: whatwg/html#10344 Bug: 1521345 Fixed: 343302069, 343093082, 343473478 Change-Id: If6308a67bac9050f695d18d275ea86c23ac22b0d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5235516 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1309009}
Allowing the page to call showPicker() on select elements as much as it wants without consuming user activation may result in the user being unable to interact with the browser UI due to popups always taking focus. The HTML spec also says to do this for input elements, so I added code to do it there as well. This patch also modified the select showPicker test because calling showPicker on a select twice in a row in the test somehow resulted in blink not seeing any input events on the second test_driver.bless(), perhaps because the select's popup is still open and is somehow intercepting the input. HTML spec: whatwg/html#10344 Bug: 1521345 Fixed: 343302069, 343093082, 343473478 Change-Id: If6308a67bac9050f695d18d275ea86c23ac22b0d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5235516 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1309009}
…er, a=testonly Automatic update from web-platform-tests Show the picker consumes activation Spec PR: whatwg/html#10344. -- wpt-commits: ad94245219488193c22568dfc95760fd0f3a413e wpt-pr: 46502
More in-depth alternative to #10098. Fixes #10084, helps GHSA-hr74-5fj7-jgxp. cc @zcorpan @lukewarlow @josepharhar @domenic @annevk
(See WHATWG Working Mode: Changes for more details.)
/input.html ( diff )