1880685 - Non-modal panels/popups like bookmark editing, permissions and translation offers overlap & compete-with modal dialogs, e.g. default-browser popup, close-and-quit dialog

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect)

Description

[Daniel Holbert [:dholbert]]

STR:

1. Set another browser as your system default browser.
2. Start Firefox Nightly with a fresh profile, and then immediately quit.

mkdir /tmp/test-profile
firefox -profile /tmp/test-profile
[Ctrl+Q]

3. Start Firefox Nightly again, now immediately visiting some foreign-language page.

firefox -profile /tmp/test-profile "https://es.wikipedia.org/wiki/Wikipedia:Portada"

ACTUAL RESULTS:

• Two modal dialogs appear: (1) a translation dialog in the foreground, (2) a default-browser dialog, in the background.
• Even though the translation dialog appears in the foreground, it's not interactive. (Its buttons have no effect.)
• Even though the default-browser dialog appears in the background, it is interactive -- and your clicks in covered-up parts of that dialog will make it through to buttons/UI that you cannot see!! This is particularly noticeable if you hover/click the (covered-up) label for the checkbox in the background dialog, since that part is partially-visible.

EXPECTED RESULTS:

• We shouldn't show overlapping modal dialogs/notifications.
• Or, if we do show overlapping modal dialogs, we should be sure that their visual stacking order matches their interactive stacking order.

(There might be a Layout or Web Painting bug hiding here, with the paint-vs-click-event stacking ordering mismatch; CC'ing emilio & tnikkel & myself for potential thoughts/investigation there. But also: independent of that, we may want to prevent ourselves from getting into this multiple-modal-popups-at-once situation entirely, maybe, via some Messaging System logic/checks?)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screencast of bug, affecting default-browser dialog (around 0:11) as well as Ctr+Q dialog (around 0:30)

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screenshot showing stacked default-browser dialog & translation popup

Here's a screenshot from the included screencast. Note my cursor position in this screenshot -- I'm hovering the translation dialog, but in fact my mouse events are being sent to the default-browser dialog underneath of it (I just clicked at the location where the cursor is shown in this screenshot, and that resulted in the checkbox being checked in the "covered-up" dialog).

Comment 3

[Daniel Holbert [:dholbert]]

This affects the "Add Bookmark" popup, too.

STR for that, in a fresh profile (where notably Confirm before quitting with Ctrl+Q must be enabled):

1. Visit https://example.org
2. Click the star button in the URL bar.
3. Press Ctrl+Q
4. Look at the modal dialogs, see which is foreground vs. which is actually-interactive.

(Similar to in comment 0: the bookmark-star-popup is drawn in the foreground, but the Ctrl+Q confirmation dialog is the one that receives clicks, even on covered-up areas of that dialog.)

Comment 4

[Daniel Holbert [:dholbert]]

This isn't a recent regression; I think it dates back to when we made these popups be styled content rather than being separate OS-level dialogs.

I can repro in 2021-04-01, using STR from comment 3.
I cannot repro in 2021-02-01 because the confirm-quit dialog seems to appear as an actual OS-level popup-window (locked to a position in the center of the initiating window).

I can't easily further-bisect in that range since mozregression-launched Firefox refuses to show me the Ctrl-Q popup even if I ask it to, but it looks like this probably came from the proton-modals work, possibly bug 1685313 or something related to that.

Comment 5

[Daniel Holbert [:dholbert]]

I confirmed I can reproduce on macOS using the comment 3 STR with current Nightly, btw. (Up until now I've been testing on Linux -- specifically, Ubuntu 22.04 with Wayland.)

Bisecting a bit further beyond comment 4 (on Linux): Nightly 2021-02-16 is "good" and Nightly 2021-02-17 is "bad".

That gives this regression pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c1db635de2e140dcf35e5a93e0e8fd94fa7474f3&tochange=6d7590bfd8d37fd1088f8d184702238881fc048b

It's not obvious to me what in there would be responsible, though. Bug 1681356 seems conceivably-related but it's Linux-specific which probably means it can't be related (since per above, this repro's on multiple platforms).

Comment 6

[Daniel Holbert [:dholbert]]

I get that same regression-range on macOS, too. I bisected it further since there was one more Nightly available (built from 3d42785f84cb4251378b1cd3583df18fd8aa8bd9 , which is "bad"), and narrowed it to this smaller range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c1db635de2e140dcf35e5a93e0e8fd94fa7474f3&tochange=3d42785f84cb4251378b1cd3583df18fd8aa8bd9

Comment 7

[Daniel Holbert [:dholbert]]

In that range, I suspect bug 1693061 is most-related. The first comment starts out with "When a window-modal dialog like the multi-tab quit warning shows up [...]" which is part of the setup here as well. --> Tentatively flagging as a regression from that bug.

The patch there was pretty small (reducing the amount of invalidation when -moz-user-input changes); maybe we were inadvertently depending on (or benefiting from) the old/greedier invalidation, in ditching existing popups when a modal dialog appears?

Comment 8

[Emilio Cobos Álvarez [:emilio]]

I think the non-interactive weirdness you're seeing are an unfortunate interaction between modal dialogs (which cause the rest of the page to be inert) and menupopups. Adding moz-inert to the menupopup rule in xul.css will probably fix it

Comment 9

[Daniel Holbert [:dholbert]]

Thanks!

Side note, this looks very similar to bug 1698066 and bug 1694811 (which appear to be dupes of each other).

Maybe the popups in this bug here (from translation/bookmark UI in URLbar) and the ones that were fixed in bug 1694811 should all be avoiding this issue in the same way (whether via the logic added in bug 1694811, or via emilio's suggestion here in comment 8).

Comment 10

[Daniel Holbert [:dholbert]]

I might be misunderstanding the suggestion in comment 8, but FWIW I tested this diff and it doesn't seem to help, in my local debug build with STR in comment 3:

diff --git a/toolkit/content/xul.css b/toolkit/content/xul.css
--- a/toolkit/content/xul.css
+++ b/toolkit/content/xul.css
@@ -358,6 +358,9 @@ menubar > menu:empty {
 
 /********* menupopup, panel, & tooltip ***********/
 
+menupopup {
+  -moz-inert: inert;
+}
 menupopup,
 panel {
   flex-direction: column;

Comment 11

[Emilio Cobos Álvarez [:emilio]]

You want -moz-inert: none; on top of the flex-direction: column

Comment 12

[Emilio Cobos Álvarez [:emilio]]

(that should override the inert value from the root element)

Comment 13

[Timothy Nikkel (:tnikkel)]

One could verify the display lists used for paint vs event targeting by dumping the display list for both. To dump the event display list set gDumpEventList to true in nsLayoutUtils.cpp. To dump the painting display list set the pref layout.display-list.dump to true. You'll get a lot of output but should be easy to answer the question.

Comment 14

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1880685 - Remove now useless rule for popups. r=#theme,Gijs

Menupopups no longer use XUL / flex layout since bug 1809084.

Comment 15

[Emilio Cobos Álvarez [:emilio]]

Uh, there was supposed to be a follow-up commit to that, ni? to submit it when I'm back in the keyboard.

Comment 16

[Emilio Cobos Álvarez [:emilio]]

So, the behavior pre-regression presumably was that the popup got reframed (and thus hidden due to nsMenuPopupFrame::DestroyFrom hiding the popup). So this never worked intentionally, was always kind of by chance.

Comment 17

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1880685 - Make popups interactable even when a modal dialog is shown. r=Gijs

Otherwise they are inert (which implies pointer-events: none), but since
the popup painting escapes the usual rules it looks off.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ba4296bee03
Remove now useless rule for popups. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/afbbee46dfa8
Make popups interactable even when a modal dialog is shown. r=Gijs

Comment 19

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/5ba4296bee03
https://hg.mozilla.org/mozilla-central/rev/afbbee46dfa8

Comment 20

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1693061

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox124 to wontfix.

For more information, please visit BugBot documentation.

Comment 22

[Marius Coman [:mcoman], Ecosystem QA, PTO until 20th of October, for any testing requests ping :poiegas]

I have verified that this issue is no longer reproducible using the latest Firefox Nightly 125.0a1 (Build ID: 20240227095754) installed on Windows 10 x64, macOS 14.1.1, and Ubuntu 22.04 x64. I can confirm the following:

• Both the "Translation" doorhanger and the "Default browser" prompt are actionable if are displayed simultaneously.
• Both the "Bookmarks" doorhanger and the "Confirm before quitting" prompt are actionable if are displayed simultaneously.