After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 779974 - Turning notification banners off doesnt turn off all notification banners
Turning notification banners off doesnt turn off all notification banners
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: message-tray
3.22.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2017-03-13 11:20 UTC by Bastian Ilsø
Modified: 2017-10-26 17:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
zotero notification banner (9.06 KB, image/png)
2017-03-13 11:20 UTC, Bastian Ilsø
  Details
messageTray: Ignore showBanners policy for critical notifications (1.24 KB, patch)
2017-03-13 19:55 UTC, Florian Müllner
committed Details | Review
windowAttentionHandler: Follow app policy for attention notifications (1.42 KB, patch)
2017-10-26 16:21 UTC, Florian Müllner
committed Details | Review

Description Bastian Ilsø 2017-03-13 11:20:49 UTC
Created attachment 347820 [details]
zotero notification banner

I have notification banners turned off inside my notification settings. But I still see this banner sometimes:

Zotero
"Software Update" is ready


I think this banner is triggered because this application searches for software updates in the background and spawns a dialog on a different workspace or while unfocused without any action done on my part. Anyway, doesnt turning notification banners off that i shouldnt see any banners at all? I would assume even banners like these should just go straight to the message tray.
Comment 1 Florian Müllner 2017-03-13 15:22:15 UTC
Right, this is a banner from gnome-shell itself, after the app was prevented from stealing focus. This is an easy fix, we just have to figure out what behavior we want:

 - follow the "master" notification settings, that
   is the "Notification Popups" switch in Settings

 - follow the per-app settings for the corresonding
   app
Comment 2 Allan Day 2017-03-13 15:32:46 UTC
(In reply to Bastian Ilsø from comment #0)
... 
> I have notification banners turned off inside my notification settings. But
> I still see this banner sometimes:
> 
> Zotero
> "Software Update" is ready

Just because someone has notifications turned off doesn't necessarily mean that they don't want any notifications to ever be displayed. Critical power warnings come to mind.

In your case, do you definitely not want to see that notification? Can you explain why not?
Comment 3 Florian Müllner 2017-03-13 15:48:24 UTC
(In reply to Allan Day from comment #2)
> Just because someone has notifications turned off doesn't necessarily mean
> that they don't want any notifications to ever be displayed. Critical power
> warnings come to mind.

This makes sense, but a quick peek at the code suggests that right now we are in fact applying the policy to critical notifications like power warnings :-)

Maybe the "master" setting shouldn't be a simple switch, but a slider from "none" over "only critical" to "all"? I'm a bit wary of exempting all critical notifications from any policy, as it provides some incentive to apps to abuse the hint for getting around restrictions ...

In any case it's a different issue than the one raised in the bug:
the notifications in question aren't critical at all, they just get a free pass for being emitted by gnome-shell itself rather than some other component.
Comment 4 Allan Day 2017-03-13 19:00:47 UTC
(In reply to Florian Müllner from comment #3)
... 
> Maybe the "master" setting shouldn't be a simple switch, but a slider from
> "none" over "only critical" to "all"? I'm a bit wary of exempting all
> critical notifications from any policy, as it provides some incentive to
> apps to abuse the hint for getting around restrictions ...
...

I'm not sure how you avoid that. There are some notifications that you just want to see, no matter the situation - an alarm, an incoming call, the fact that you're almost out of power...
Comment 5 Florian Müllner 2017-03-13 19:55:07 UTC
Created attachment 347869 [details] [review]
messageTray: Ignore showBanners policy for critical notifications

The critical hint is meant to be used for notifications that must not
be missed - running out of battery being the prime example - so it
makes sense to ignore the policy in that case and make sure to always
show them to the user. This is consistent with blocking normal
notifications while showing a fullscreen window, but letting critical
ones through.

(In reply to Allan Day from comment #4)
> There are some notifications that you just want to see, no matter
> the situation

Poor Bastian, asking for *less* notifications, and we are showing him *more* ...
Comment 6 Rui Matos 2017-03-13 23:11:24 UTC
Review of attachment 347869 [details] [review]:

Sure.

But what about applying a policy to the WindowAttentionSource? It seems like we could use a NotificationApplicationPolicy there.
Comment 7 Florian Müllner 2017-03-13 23:17:09 UTC
(In reply to Rui Matos from comment #6)
> But what about applying a policy to the WindowAttentionSource? It seems like
> we could use a NotificationApplicationPolicy there.

Yeah, or a NotificationGenericPolicy, see comment #1. I'm unsure which is the better option, as those notifications don't really come from the app.
But I don't think they are so indispensable that they have to be shown at all cost, regardless of any user settings.
Comment 8 Florian Müllner 2017-03-13 23:24:24 UTC
Comment on attachment 347869 [details] [review]
messageTray: Ignore showBanners policy for critical notifications

Attachment 347869 [details] pushed as 60a2794 - messageTray: Ignore showBanners policy for critical notifications
Comment 9 Florian Müllner 2017-10-26 16:21:57 UTC
Created attachment 362358 [details] [review]
windowAttentionHandler: Follow app policy for attention notifications

While window attention notifications are created by the shell itself
rather than applications (most likely as a result of focus stealing
prevention), users still commonly link them to the application for
which they are shown. It makes therefore sense to follow the appropriate
policy set by the user rather than showing them unconditionally.
Comment 10 Rui Matos 2017-10-26 17:11:37 UTC
Review of attachment 362358 [details] [review]:

otherwise makes sense. didn't test

::: js/ui/windowAttentionHandler.js
@@ +82,3 @@
+    _createPoliy: function() {
+        if (this._app && this._app.get_app_info()) {
+            let id = this.app.get_id().replace(/\.desktop$/,'');

this.app doesn't seem to exist in either this class or the parent. Do you mean this._app.get_app_info().get_id() ?
Comment 11 Florian Müllner 2017-10-26 17:40:56 UTC
Attachment 362358 [details] pushed as eecbd4d - windowAttentionHandler: Follow app policy for attention notifications

(In reply to Rui Matos from comment #10)
> this.app doesn't seem to exist in either this class or the parent. Do you
> mean this._app.get_app_info().get_id() ?

Yeah, I meant this._app.get_id() ...
Comment 12 Alessandro Bono 2017-10-26 17:52:53 UTC
Review of attachment 362358 [details] [review]:

::: js/ui/windowAttentionHandler.js
@@ +80,3 @@
     },
 
+    _createPoliy: function() {

There is a typo. It should be `_createPolicy`.