1907032 - Don't allow opening <select> picker in background tabs

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Tom Schuster (MoCo) | OOO back 1. Dec]

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.

Comment 1

[Daniel Veditz [:dveditz]]

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.

Comment 2

[Tom Schuster (MoCo) | OOO back 1. Dec]

Attachment: Bug 1907032 - Don't show the select picker in active tabs. r?canadahonk

Comment 3

[Tom Schuster (MoCo) | OOO back 1. Dec]

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.

Comment 4

[Tom Schuster (MoCo) | OOO back 1. Dec]

I actually I think bug 1909163 might fix this in a better way? It sounds quite similar.

Comment 5

[Tom Schuster (MoCo) | OOO back 1. Dec]

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

[:Gijs (he/him)]

(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 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/adadd4d9db34
Don't show the select picker in active tabs. r=emilio

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/adadd4d9db34

Comment 9

[:Gijs (he/him)]

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?

Comment 10

[Tom Schuster (MoCo) | OOO back 1. Dec]

(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.

Comment 11

[Tom Schuster (MoCo) | OOO back 1. Dec]

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):

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/e956139cab26

Comment 14

[Tom Schuster (MoCo) | OOO back 1. Dec]

I am going to see if I can write a test for this.

Comment 15

[:Gijs (he/him)]

(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. :-)

Comment 16

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/f923b729ccbf

Comment 17

[Daniel Veditz [:dveditz]]

The SelectElement didn't support showPicker() until bug 1854112