Don't allow opening <select> picker in background tabs
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
People
(Reporter: tschuster, Assigned: tschuster)
References
Details
(Keywords: sec-want, Whiteboard: [adv-main130-][adv-esr128.2-])
Attachments
(1 file)
|
48 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr128+
|
Details | Review |
One thing that confused me about some of the <select> issues is why at least switching away from the tab doesn't close the <select> dropdown. I realized that the showPicker function works even if the <select> is in a background tab!
We do check for IsRendered, but we also need something like BrowsingContext::IsActive (or something similar, nsPIDOMWindowInner::IsFullyActive at least seems to not work).
I think this code in SelectChild.sys.mjs is supposed to close the dropdown on pagehide, we should make sure that works correctly.
So far I haven't been able to reproduce this issue with <input type=date> pickers.
Updated•2 years ago
|
Comment 1•2 years ago
|
||
This would address several security bugs / tricks that already have a security rating so we can call this "sec-want" rather than doubling up on sec-moderate or whatever.
| Assignee | ||
Comment 2•2 years ago
|
||
Updated•2 years ago
|
| Assignee | ||
Comment 3•2 years ago
|
||
This patch is more of a proof of concept. I imagine we want to try to get something like this standardized as well. Probably we could extend Step 4. " .. this is not being rendered .." with "is not in active tab" or rather whatever is the closest comparable check in HTML standard terms.
I won't be able to finish this completely right now.
| Assignee | ||
Comment 4•2 years ago
|
||
I actually I think bug 1909163 might fix this in a better way? It sounds quite similar.
Updated•2 years ago
|
| Assignee | ||
Comment 5•2 years ago
|
||
Gijs, do you know why even with bug 1909163, opening a dropdown for a tab in background would still show it on the current active tab? "tabspecific" sounds like it should fix this, maybe? testcase
Comment 6•2 years ago
|
||
(In reply to Tom Schuster (MoCo) from comment #5)
Gijs, do you know why even with bug 1909163, opening a dropdown for a tab in background would still show it on the current active tab? "tabspecific" sounds like it should fix this, maybe? testcase
tabspecific just means the popup is closed when the selected tab changes. Cf. https://searchfox.org/mozilla-central/rev/b1b87f95ecea00828298d1b3cd3d8718f9fcc3fc/browser/base/content/browser.js#3367-3371
Comment 8•2 years ago
|
||
Comment 9•2 years ago
|
||
I'm being asked about uplift to esr128 in bug 1909163; I'm guessing we'll want this on 130 and esr128 as well, then?
| Assignee | ||
Comment 10•2 years ago
|
||
(In reply to :Gijs (he/him) from comment #9)
I'm being asked about uplift to esr128 in bug 1909163; I'm guessing we'll want this on 130 and esr128 as well, then?
Yeah, I guess that makes sense. This is simple enough and might make some attacks harder or impossible.
| Assignee | ||
Comment 11•2 years ago
|
||
Comment on attachment 9413189 [details]
Bug 1907032 - Don't show the select picker in active tabs. r?canadahonk
Beta/Release Uplift Approval Request
- User impact if declined: A malicious page can show an overlapping select dropdown even after navigating away from the page.
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Relatively simple one line change in an API that is probably not widely used yet. (No Safari support)
- String changes made/needed:
- Is Android affected?: Unknown
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration:
- User impact if declined:
- Fix Landed on Version:
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky):
Updated•2 years ago
|
Comment 12•2 years ago
|
||
Comment on attachment 9413189 [details]
Bug 1907032 - Don't show the select picker in active tabs. r?canadahonk
Approved for 130.0b9 and 128.2esr.
Updated•2 years ago
|
Comment 13•2 years ago
|
||
| uplift | ||
| Assignee | ||
Comment 14•2 years ago
|
||
I am going to see if I can write a test for this.
Comment 15•2 years ago
|
||
(In reply to Tom Schuster (MoCo) from comment #14)
I am going to see if I can write a test for this.
There's already a test waiting for your review in bug 1909901. :-)
Updated•2 years ago
|
Updated•2 years ago
|
Comment 16•2 years ago
|
||
| uplift | ||
| Assignee | ||
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Comment 17•2 years ago
|
||
The SelectElement didn't support showPicker() until bug 1854112
Updated•1 year ago
|
Description
•