1869796 - XUL notification steals focus when triggered

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P2)

Description

[Nazar Mokrynskyi]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:122.0) Gecko/20100101 Firefox/122.0

Steps to reproduce:

Use anything on the web that uses notifications. Slack, Google Calendar, literally anything. It worked properly until a few weeks ago, now it is basically broken.

Actual results:

Notification does show up, but it is not rendered, instead it takes the same background as an app below it, so it is essentially invisible. I can see that after switching virtual desktops, in which case it is re-painted with previous virtual desktop's screen contents. It is also not clickable anymore

Expected results:

Should render its contents properly and be clickable.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Toolkit::Alerts Service' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Kagami Rosylight [:saschanaz] (they/them)]

(In reply to Nazar Mokrynskyi from comment #0)

Notification does show up, but it is not rendered, instead it takes the same background as an app below it, so it is essentially invisible.

Hi reporter, can you take a screenshot of this, assuming it's at least partly visible in certain situation? Also, does your system use libnotify? Thanks!

Comment 3

[Nazar Mokrynskyi]

Attachment: firefox-buggy-notification-screenshot.png

The screen I was on had black background in notification area. Then I switched to a different virtual screen and saw this.

Notifications from Firefox look nothing like notify-send notifications, so I don't think it uses libnotify.

I'm using a custom DE with Qtile window manager and Compton compositor on X11.

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

Hmm, sounds much like a widget/gtk thing. Hi GTK people, does anything come to your mind from comment #3?

Comment 5

[Kagami Rosylight [:saschanaz] (they/them)]

(In reply to Nazar Mokrynskyi from comment #3)

Notifications from Firefox look nothing like notify-send notifications, so I don't think it uses libnotify.

How did it look when it worked? Can you try the stable channel?

Comment 6

[Nazar Mokrynskyi]

When it worked it looked like a gray rectangle (I use dark theme) with icon, text and close button in top right corner. Somewhat similar to notifications Thunderbird is still showing to me (except in Thunderbird is always Thunderbird's icon and it shows up in bottom right rather than top right corner).

Comment 7

[Kagami Rosylight [:saschanaz] (they/them)]

It's hard to see from the text descriptions that it's XUL based or libnotify. 🥲

When you try with the stable channel, does it have the same issue? (In other words, is there a chance that this is not a Firefox side regression?)

Comment 8

[Nazar Mokrynskyi]

Attachment: firefox-correct-notification-screenshot.png

Firefox 120.0.1 stable works fine, notification looks like on attached image. Must be nightly regression.

Comment 9

[Kagami Rosylight [:saschanaz] (they/them)]

Okay, that's definitely XUL, thanks! Can you try pip install mozregression && mozregression --good 2022-12-13 --bad 2023-12-13 to bisect the culprit?

Comment 10

[Nazar Mokrynskyi]

mozregression is a pretty cool bisection tool!

4:17.75 INFO: Last good revision: 5f6fad561a65b0f9103350cccffd4fec374b214c
4:17.75 INFO: First bad revision: 2c0e51ad033428fa026ec754cccd91c9ae4f2a26
4:17.75 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5f6fad561a65b0f9103350cccffd4fec374b214c&tochange=2c0e51ad033428fa026ec754cccd91c9ae4f2a26

Comment 11

[Kagami Rosylight [:saschanaz] (they/them)]

Okay, that says this is a regression from bug 1864382. Thank you very much to provide all the details!

Comment 12

[Martin Stránský [:stransky] (ni? me)]

Thanks, will look at it. Looks like a child window misconfiguration related to mContainer.

Comment 13

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

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

Comment 14

[Donal Meehan [:dmeehan]]

(In reply to Martin Stránský [:stransky] (ni? me) from comment #12)

Thanks, will look at it. Looks like a child window misconfiguration related to mContainer.

:stransky any updates on this? We are in beta for Fx122 but can take an uplift.
Could you add a severity when you have a moment?

Comment 15

[Martin Stránský [:stransky] (ni? me)]

Will looking at it. We use this notification as a fallback is libnotify is missing (which is not expected - I have no idea why someone removes it?).
As a workaround you can install libnotify package which is available for all distros and should be there by default.

Comment 16

[Martin Stránský [:stransky] (ni? me)]

Nazar, just out of curiosity, which distro/system do you use? Did you removed libnotify intentionally?

Comment 17

[Nazar Mokrynskyi]

I am on Ubuntu 22.04 right now and I have both libnotify-bin and libnotify4 installed and always had them.
No idea why Firefox and Thunderbird do not use them, but it was like this at least for a few years as I'm using Firefox Nightly and Thunderbird Beta versions that are direct from Mozilla (not packaged in deb).

Comment 18

[Nazar Mokrynskyi]

Notification started to work properly again in latest Nightly build I just installed

Comment 19

[Nazar Mokrynskyi]

Okay, there is one big regression comparing to before.
Notifications now capture focus. So if I'm typing something and notification happens, focus is taken away and I need to click on the text field to continue typing.
This is a massive usability issue, extremely counter-productive.

Comment 20

[Nazar Mokrynskyi]

And looks like Thunderbird updated to newer base and 122.0b1 is having the same notification issue there now.

Comment 21

[Kagami Rosylight [:saschanaz] (they/them)]

Notification started to work properly again in latest Nightly build I just installed

👀 Can you use mozregression to see which build caused the (good) change?

Comment 22

[Nazar Mokrynskyi]

This one: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d75adfaf9dd069655754ef9e0d6510bf16f506a0&tochange=7f3e10bdd2af504b2affd3847c27a4ec3b8ca8a2

But I think window doesn't have proper classes on it, which makes it capture focus and only show on one virtual desktop instead of all as normal notifications do.

So it is a step into the right direction, but not a proper fix.

Comment 23

[Kagami Rosylight [:saschanaz] (they/them)]

Thanks! Emilio, care to take a look per comment #22?

We still need to know why Firefox is failing to detect libnotify and use XUL at all, though.

Comment 24

[Kagami Rosylight [:saschanaz] (they/them)]

Nazar, can you check the value of alerts.useSystemBackend in about:config? If this is false then it always uses XUL.

Comment 25

[Nazar Mokrynskyi]

alerts.useSystemBackend is true and always was that way, I already checked it during initial debugging

Comment 26

[Emilio Cobos Álvarez (:emilio)]

First of all, is there something in dmesg or the journal the first time a notification is received? It should probably highlight what's going on with libnotify. Alternatively if you have a build, looking at here should help figure out what's up.

I guess the focus stuff could be explained for this. It's unclear to me that was ever meant to apply to top-level windows, but it probably did.

We'd need to plumb stuff through a little differently to support that.

Comment 27

[Nazar Mokrynskyi]

First of all, is there something in dmesg or the journal the first time a notification is received?

Nothing in kernel logs, nothing in logs firefox prints in terminal either.

Comment 28

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1869796 - Minor nsWindow clean-ups. r=stransky

nsBaseWidget already stores these / exposes IsPIPWindow().

Comment 29

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1869796 - Use shifts to define chrome flags. r=smaug

This makes clearer which bits are free and so on.

Depends on D197866

Comment 30

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1869796 - Fix some clang-tidy warnings in nsAppShellService. r=smaug,stransky

Depends on D197867

Comment 31

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1869796 - Remove some unused chrome flags. r=smaug

And rename CHROME_WINDOW_MIN to CHROME_WINDOW_MINIMIZE, for parallel
with the resize flag.

Depends on D197868

Comment 32

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1869796 - Clean-up a bit CalculateWindowZLevel. r=smaug

CHROME_MODAL implies CHROME_DEPENDENT.

Depends on D197869

Comment 33

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1869796 - Inline CalculateChromeFlagsHelper. r=smaug

The only caller already is calculating a lot of other flags.

Depends on D197870

Comment 34

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1869796 - Make xul alert windows have the appropriate type hint. r=stransky

Before bug 1869796 they had an utility type hint, now they have a
toplevel type hint, after the patch they have a notification type hint,
which seems more appropriate.

Depends on D197871

Comment 35

[Kagami Rosylight [:saschanaz] (they/them)]

Are the patches for comment #19? Should the bug title change?

Comment 36

[Emilio Cobos Álvarez (:emilio)]

Yes, and maybe? The title is rather general but comment 19 is still "not working properly" :)

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d209e8f5b754
Minor nsWindow clean-ups. r=stransky

Comment 38

[Noemi Erli[:noemi_erli]]

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

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9fe33a9242c6
Use shifts to define chrome flags. r=smaug
https://hg.mozilla.org/integration/autoland/rev/3af518535720
Fix some clang-tidy warnings in nsAppShellService. r=stransky
https://hg.mozilla.org/integration/autoland/rev/8fe5f47abacb
Remove some unused chrome flags. r=smaug,sessionstore-reviewers,dao

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9201220ea63a
Inline CalculateChromeFlagsHelper. r=smaug

Comment 41

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6a4793cef01a
Make xul alert windows have the appropriate type hint. r=stransky

Comment 42

[Donal Meehan [:dmeehan]]

:emilio this hasn't landed in central yet, but we are in the final week of beta for Fx122 so reaching out about uplifts.
Any concerns with fixing this in Fx122? Will it be safe to uplift for beta?

Comment 43

[Emilio Cobos Álvarez (:emilio)]

So the real fix for rendering of the notification is in bug 1870512. This only fixes the focus issue afterwards.

Given that, I think we'd have to uplift bug 1870512. The focus issue seems probably bearable for the rare-ish case non-native notifications are used.

Comment 44

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/9fe33a9242c6
https://hg.mozilla.org/mozilla-central/rev/3af518535720
https://hg.mozilla.org/mozilla-central/rev/8fe5f47abacb

Comment 45

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/9201220ea63a
https://hg.mozilla.org/mozilla-central/rev/6a4793cef01a

Comment 46

[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-firefox122 to wontfix.

For more information, please visit BugBot documentation.

Comment 47

[Nazar Mokrynskyi]

I'm using the latest Nightly as of right now (123.0a1 (2024-01-11)) and the window still steals focus just like before, did the fix actually land or did anyone test that it works as expected?

Comment 48

[Nazar Mokrynskyi]

Any updates? 123 was still broken despite tagged as fixed, 124 arrived yesterday, still broken the same way.

Comment 49

[Kagami Rosylight [:saschanaz] (they/them)]

Do you want to double check?

Comment 50

[Emilio Cobos Álvarez (:emilio)]

(In reply to Nazar Mokrynskyi from comment #48)

Any updates? 123 was still broken despite tagged as fixed, 124 arrived yesterday, still broken the same way.

Your WM is probably giving focus to the window even though it has a notification role (since the role changed from "utility" to notification).

Comment 51

[Emilio Cobos Álvarez (:emilio)]

Let's file a separate bug to track that, but the real thing to fix here is bug 1873384, IMO.

Comment 52

[Kagami Rosylight [:saschanaz] (they/them)]

I'm using the latest Nightly as of right now (123.0a1 (2024-01-11)) and the window still steals focus just like before, did the fix actually land or did anyone test that it works as expected?

I can confirm similar behavior (if not same) on Windows and filed bug 1878037.

Comment 53

[Nazar Mokrynskyi]

So the issue is definitely not fixed and should be reopened. The already landed fix is almost complete though, according to discussion at https://github.com/qtile/qtile/issues/4662 "override-redirect" attribute is missing that will fix the issue fully.

Comment 54

[Emilio Cobos Álvarez (:emilio)]

Let's track that in bug 1882033