signalapp / Signal-Desktop

A private messenger for Windows, macOS, and Linux.
https://signal.org/download
GNU Affero General Public License v3.0
14.61k stars 2.66k forks source link

Linux: window.focus/window.show triggers useless "Signal is ready" on every notification, when using GNOME #5004

Open diegoe opened 3 years ago

diegoe commented 3 years ago

Bug Description

Mutter/GNOME Shell have, since forever, prevented focus stealing. Instead, a notification of "App is ready" is shown, this is helpful when you start an app, and go do something else while it opens (think your browser, or -hah- an electron app). Also super helpful to avoid the exasperation of apps stealing focus for whatever reason.

However, whenever Signal sends a message notification, it seems to try to raise the window when "Draw attention to this window" is checked. This causes any incoming message's notification to be immediately obstructed by a "Signal is ready" notification (see screenshot).

https://github.com/signalapp/Signal-Desktop/blob/a173a9b33ff8dccbe02b9ad2d5e268ca3b036563/js/notifications.js#L158-L165 https://github.com/signalapp/Signal-Desktop/blob/7095e070eb540d2ea2af42b489e3d6c2a4b8175e/main.js#L124-L146

I think .focus() vs .show() did not make a difference for me, but I could be misremembering.

Arguably, you could say that it is mutter's choice to interpret focus/show as an event itself (and turn it into a notification), but I would say that Signal still shouldn't be showing/focusing the window on every notification. If the goal is/was to trigger a platform's animation/behavior (blinking, bouncing, etc), then I would say that Linux (GNOME) is worthy of the same consideration.

If you would like to keep it simple, you could just default the pref to false on platform=Linux. Otherwise, you can special case GNOME (there's an env var: XDG_CURRENT_DESKTOP). Note that default GNOME has no tray or window list, and no bouncing/blinking/etc behavior except the "is ready" notification.

Steps to Reproduce

  1. Receive a notification
  2. See your message notification immediately get obscured by "Signal is ready"
  3. Be sad

Actual Result:

Notifications are obscured because of the window.focus()/window.show() triggered with each notification

Expected Result:

Signal triggers only one notification (the message one)

Screenshots

Screenshot from 2021-01-29 14-36-27

Platform Info

Signal Version:

1.39.6, through Flatpak

Operating System:

Debian sid + GNOME 3.38 (can reproduce on HEAD, too)

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 3 years ago

This issue has been closed due to inactivity.

Forage commented 3 years ago

Please open this issue again since it is all but fixed. It causes the real notification of the received message to be masked. The RocketChat Electron based desktop application suffered from the same issue and recently got it fixed so https://github.com/RocketChat/Rocket.Chat.Electron/pull/2122 might serve as inspiration to fix it in Signal as well.

In my case I'm on Ubuntu 21.04 using the deb Signal 5.18.0 package.

yochaigal commented 2 years ago

Can confirm the same issue on Ubuntu 20.04. I use the "hide update is ready" extension and it works fine (though it still steals focus!).

Forage commented 2 years ago

Can confirm the same issue on Ubuntu 20.04. I use the "hide update is ready" extension and it works fine (though it still steals focus!).

The focus stealing can be avoided when unchecking the option "Draw attention to this window when a notification arrives" in "File / Preferences / Notifications"

maymage commented 1 year ago

Can confirm the same issue on Ubuntu 20.04. I use the "hide update is ready" extension and it works fine (though it still steals focus!).

The focus stealing can be avoided when unchecking the option "Draw attention to this window when a notification arrives" in "File / Preferences / Notifications"

not really

nekohayo commented 8 months ago

Here is the upstream issue where Electron has not yet recognized the incorrect approach: https://github.com/electron/electron/issues/18445