GNOME Bugzilla – Bug 608667
Notifications does not show in bottom, if any window in full-screen mode
Last modified: 2011-06-17 22:22:46 UTC
reproducible: always steps to reproduce: open firefox, press F11, make something to get notification.
Jon? Currently this is accidental. It could be changed. It may make sense to show some notifications (urgent?) but not others? And regardless of whether or not we actively notify, can the user force the summary area to come up? (Note that the Activities hotspot is not active in full-screen mode either.)
Feb 01 19:09:12 <mccann> I guess you can make fairly good arguments for either Feb 01 19:09:48 <fmuellner> mccann: yup ... in the case of firefox, i'd probably want the notification Feb 01 19:09:53 <mccann> are we talking about the hot edge or showing banners at all? Feb 01 19:09:58 <fmuellner> in the case of totem etc probably not Feb 01 19:10:07 <mccann> i guess i very rarely go full screen except in a movie Feb 01 19:10:43 <fmuellner> i think it's about showing incoming notifications Feb 01 19:12:43 <fmuellner> mccann: yup, no notifications when in fullscreen mode Feb 01 19:14:36 <mccann> I'm not sure i like special casing fullscreen windows that much. Might be better to decide based on busy vs. not-busy. And maybe some apps can automatically set you as busy when in fullscreen (if that is the only monitor)? Feb 01 19:14:46 <mccann> I'd have to think about it Feb 01 19:15:49 <mccann> I guess the current behavior is that any fullscreen window implicitly marks the session as busy Feb 01 19:16:13 <mccann> but "urgent" notifications would have to come through anyway Feb 01 19:16:46 <mccann> say the battery warning. In fact it is particularly important here because I can't see my battery indicator Feb 01 19:18:30 <mccann> but does it mean that I want to ignore a chat from a friend if I'm in fullscreen in inkscape or firefox? I'd say probably not. Watching a movie? Sometimes yes sometimes no. In fact the other day I was chatting while watching a movie. Feb 01 19:19:25 <fmuellner> mccann: mmmh, makes sense ... problem of course: atm we _are_ special casing fullscreen windows ... Feb 01 19:19:26 <mccann> I guess if I'm not interested in doing that I can a) mark myself as busy proactively b) have some way to mark myself as busy from the banner Feb 01 19:20:07 <mccann> though we should probably have a way to respond to the chat politely instead of just marking as busy :) Feb 01 19:20:18 <mccann> dunno Feb 01 19:20:37 <mccann> fmuellner: so yeah if we are doing that now I'd like to hear the rationale I guess Feb 01 19:21:36 <fmuellner> mccann: oh, i think it's more or less coincidence ... technically, notifications are in the same group as the panel Feb 01 19:21:47 <mccann> basically I'm not sure it is well understood or obvious that fullscreening should stop notifications Feb 01 19:22:11 <fmuellner> mccann: the special casing was introduced for the panel Feb 01 19:22:27 <mccann> right I'm just speaking generally Feb 01 19:22:38 <mccann> from the user's perspective Feb 01 19:23:04 <fmuellner> mccann: i think from the user's perspective it's like you said Feb 01 19:23:47 <mccann> sometimes my desire is to immerse myself in something but some times it is just to get more space for what I'm doing Feb 01 19:24:12 <mccann> also, only showing the messages on the primary monitor will help too Feb 01 19:24:20 <mccann> so that they never show up on projectors Feb 01 19:24:31 <mccann> etc Feb 01 19:26:28 <mccann> fmuellner: anyway those are some random thoughts on the matter So, I think the summary is something like this: Use our busy status if you want to queue notifications. Fullscreen is just full-screen and probably shouldn't (or doesn't) imply that you don't want to be interrupted.
I am agree with William. IMHO, if user allows notifications in applications - their are all important for him. Say, icq notifications not urgent (for the gnome-shell's brain ;) , but if user waits important message? Good idea to use "busy-status" for that. P.S. I like current style of notifications!
Created attachment 159888 [details] [review] [Params] always return a new object from Params.parse Rather than returning a modified copy of the input params/defaults, always return a new object, copying the appropriate values into it.
Created attachment 159889 [details] [review] [ShellGenericContainer] add _get_skip_paint()
Created attachment 159890 [details] [review] [Chrome] redo using ShellGenericContainer Previously we used a ClutterGroup containing a second ClutterGroup for the non-visibleInOverview actors. Redo it using a single ShellGenericContainer, and use set_skip_paint() to hide the non-overview chrome when the overview is visible. Also fix up the default values for trackActor().
Created attachment 159891 [details] [review] [Chrome] add "visibleInFullscreen" property Chrome elements with this flag set will be visible even when a fullscreen window is present.
Created attachment 159892 [details] [review] [MessageTray] Make urgent notifications show up over fullscreen windows Use the chrome "visibleInFullscreen" flag, but only show urgent notifications when a fullscreen window is present. Having just reread the bug, I see that actually this is *not* the behavior we agreed on, although all the patches up to this one are still correct, and I might as well attach this one since I already wrote it and we might change our minds later. :)
Created attachment 159914 [details] [review] [MessageTray] make this visible in fullscreen mode (unlike previous patch, this just makes the message tray always fully available in fullscreen mode)
Created attachment 159915 [details] [review] [GnomeSession] split out the gnome-session presence D-Bus interface Split this out of js/ui/statusMenu.js
Created attachment 159916 [details] [review] [MessageTray] block non-urgent notifications when the session is Busy
Review of attachment 159888 [details] [review]: Looks good.
Review of attachment 159889 [details] [review]: Looks good. One idea is to replace currently_skipping in _set_skip_paint() with a call to this function.
Review of attachment 159890 [details] [review]: Looks good.
Review of attachment 159891 [details] [review]: Looks good, aside from the comments below. ::: js/ui/chrome.js @@ +16,3 @@ +const Visibility = { + FULL: 1, Should we start enumerations with 0 value? (Seems like we have both types of precedents in the code.) @@ +41,3 @@ + this._inFullscreen = false; + this._inOverview = false; + this.visibility = Visibility.FULL; this._inFullscreen and this._inOverview seem to be redundant with this.visibility; can we just use this.visibility and pass the new mode into _updateVisibility() ?
Review of attachment 159914 [details] [review]: Looks good. Works as expected.
Review of attachment 159915 [details] [review]: Looks good. ::: js/misc/gnomeSession.js @@ +44,3 @@ +}; +DBus.proxifyPrototype(Presence.prototype, PresenceIface); + Not sure it's such a crime, but 'git bz apply' complained that there is a blank line at EOF.
Review of attachment 159916 [details] [review]: We also need to check notificationsBlocked before showing the message tray because the set of sources in it has changed. So the relevant line should be: if (notificationsDone && this._summaryNeedsToBeShown && !notificationsBlocked) This will result in us not showing the tray after urgent notifications as well, but I think it should be ok. Otherwise we need to keep a flag about that and make it a special case. It won't be right to be changing this._summaryNeedsToBeShown variable based on the busy mode, because the busy mode might change by the time we'll need to use that variable. Good to commit when you address these comments. ::: js/ui/messageTray.js @@ +700,3 @@ + _presenceStatusChanged: function(presence, status) { + this._busy = (status == GnomeSession.PresenceStatus.BUSY); + this._updateState(); Is it worth checking here if this._busy really changed before calling this._updateState()? @@ +712,3 @@ + let notificationsBlocked = this._busy; + let notificationsPending = this._notificationQueue.length > 0 && + (!notificationsBlocked || this._notificationQueue[0].urgent); Perhaps it's worth a comment here that we move urgent notifications to the front of the queue, so we won't miss an urgent notification if we check whether the first notification in the queue is urgent.
(In reply to comment #2) > Feb 01 19:09:12 <mccann> I guess you can make fairly good arguments for > either > Feb 01 19:09:48 <fmuellner> mccann: yup ... in the case of firefox, i'd > probably want the notification > Feb 01 19:09:53 <mccann> are we talking about the hot edge or showing > banners at all? > Feb 01 19:09:58 <fmuellner> in the case of totem etc probably not > Feb 01 19:10:07 <mccann> i guess i very rarely go full screen except in a > movie > Feb 01 19:10:43 <fmuellner> i think it's about showing incoming > notifications > Feb 01 19:12:43 <fmuellner> mccann: yup, no notifications when in fullscreen > mode > Feb 01 19:14:36 <mccann> I'm not sure i like special casing fullscreen > windows that much. Might be better to decide based on busy vs. not-busy. And > maybe some apps can automatically set you as busy when in fullscreen (if that > is the only monitor)? > Feb 01 19:14:46 <mccann> I'd have to think about it > Feb 01 19:15:49 <mccann> I guess the current behavior is that any fullscreen > window implicitly marks the session as busy > Feb 01 19:16:13 <mccann> but "urgent" notifications would have to come > through anyway > Feb 01 19:16:46 <mccann> say the battery warning. In fact it is > particularly important here because I can't see my battery indicator > Feb 01 19:18:30 <mccann> but does it mean that I want to ignore a chat from > a friend if I'm in fullscreen in inkscape or firefox? I'd say probably not. > Watching a movie? Sometimes yes sometimes no. In fact the other day I was > chatting while watching a movie. > Feb 01 19:19:25 <fmuellner> mccann: mmmh, makes sense ... problem of course: > atm we _are_ special casing fullscreen windows ... > Feb 01 19:19:26 <mccann> I guess if I'm not interested in doing that I can > a) mark myself as busy proactively b) have some way to mark myself as busy from > the banner > Feb 01 19:20:07 <mccann> though we should probably have a way to respond to > the chat politely instead of just marking as busy :) > Feb 01 19:20:18 <mccann> dunno > Feb 01 19:20:37 <mccann> fmuellner: so yeah if we are doing that now I'd > like to hear the rationale I guess > Feb 01 19:21:36 <fmuellner> mccann: oh, i think it's more or less > coincidence ... technically, notifications are in the same group as the panel > Feb 01 19:21:47 <mccann> basically I'm not sure it is well understood or > obvious that fullscreening should stop notifications > Feb 01 19:22:11 <fmuellner> mccann: the special casing was introduced for > the panel > Feb 01 19:22:27 <mccann> right I'm just speaking generally > Feb 01 19:22:38 <mccann> from the user's perspective > Feb 01 19:23:04 <fmuellner> mccann: i think from the user's perspective it's > like you said > Feb 01 19:23:47 <mccann> sometimes my desire is to immerse myself in > something but some times it is just to get more space for what I'm doing > Feb 01 19:24:12 <mccann> also, only showing the messages on the primary > monitor will help too > Feb 01 19:24:20 <mccann> so that they never show up on projectors > Feb 01 19:24:31 <mccann> etc > Feb 01 19:26:28 <mccann> fmuellner: anyway those are some random thoughts on > the matter > > So, I think the summary is something like this: Use our busy status if you > want to queue notifications. Fullscreen is just full-screen and probably > shouldn't (or doesn't) imply that you don't want to be interrupted. We should do better than that ... I mean having the user select "busy" every time he starts a fullscreen app where he does not want notifications (games, movies ...) would be kind of annoying. When running a fullscreen browser window you'd probably want it, but when watching a movie or playing a game no. (some people might; but I'd say the common case is not). From a technical POV, we probably want to unredirect fullscreen windows like games for performance reasons, in this case showing notifications would be tricky,
(In reply to comment #19) > We should do better than that ... I mean having the user select "busy" every > time he starts a fullscreen app where he does not want notifications (games, > movies ...) would be kind of annoying. Yup, sounds pretty annoying. > When running a fullscreen browser window you'd probably want it, but when > watching a movie or playing a game no. This looks like a sane default, but I'm not sure whether it's a good idea to implement it in the shell itself in spite of expecting applications to use the DBus interface to adjust the status where it makes sense (just like e.g. Totem disables gnome-screensaver while playing a movie). While I think that we can often do a decent job handling it ourselves, there will always be cases where the application simply knows better. Just consider applications like Evince, which have two fullscreen modes ("Fullscreen" and "Presentation" - I'd expect notifications to be blocked only in the latter case). Adding the code to the few applications which may want to block notifications won't be a lot of work, and it's much cleaner than any method I can think of for GNOME Shell. It might make sense to add a small CLI utility similar to gnome-screensaver-command to allow scripting applications which won't support the DBus interface (e.g. wine games) - not sure though whether such a utility would live in gnome-shell or empathy ...
(In reply to comment #20) > (In reply to comment #19) > > We should do better than that ... I mean having the user select "busy" every > > time he starts a fullscreen app where he does not want notifications (games, > > movies ...) would be kind of annoying. > > Yup, sounds pretty annoying. > > > > When running a fullscreen browser window you'd probably want it, but when > > watching a movie or playing a game no. > > This looks like a sane default, but I'm not sure whether it's a good idea to > implement it in the shell itself in spite of expecting applications to use the > DBus interface to adjust the status where it makes sense (just like e.g. Totem > disables gnome-screensaver while playing a movie). > > While I think that we can often do a decent job handling it ourselves, there > will always be cases where the application simply knows better. Just consider > applications like Evince, which have two fullscreen modes ("Fullscreen" and > "Presentation" - I'd expect notifications to be blocked only in the latter > case). Yeah this one is indeed an interesting case where there are basically two full screen modes. > Adding the code to the few applications which may want to block notifications > won't be a lot of work, and it's much cleaner than any method I can think of > for GNOME Shell. It might make sense to add a small CLI utility similar to > gnome-screensaver-command to allow scripting applications which won't support > the DBus interface (e.g. wine games) - not sure though whether such a utility > would live in gnome-shell or empathy ... Yeah while indeed any heuristic we add cannot cover all cases, I am not convinced that having every fullscreen app messing with the status is a good idea either. While we should be able to fix evince, totem and some others, there are a lot of apps out of out control (like most games, other video players, flash, ...)
When thinking about showing notifications in full-screen mode, it would be nice to think how many people will be looking at the screen. If it is web browser or some text/media editor, there is probably only one person looking at it. However, if it is a media player, then more than one people will be looking at the screen. In this case, we don't want to show notifications. This will also take care of scenario like Evince in presentation mode, where one person interacts with the application while there are more than one person looking at the screen. It would also be really cool if Gnome Shell allowed one two write small scripts to control notifications in your favourite language. For example, show chat notifications if the sender is MyFriend.
(In reply to comment #15) > +const Visibility = { > + FULL: 1, > > Should we start enumerations with 0 value? (Seems like we have both types of > precedents in the code.) Wel... all the ones imported from D-Bus APIs start with 0 because someone else made that decision, probably because enums start with 0 by default in C. But since our enums don't have a default starting value, I don't think we have to feel constrained. In the (non-D-Bus) enums where we have a 0 value, it generally has definitely 0-ish sort of feeling (eg, MessageTray.State.HIDDEN, GenericDisplay.RedisplayFlags.NONE, etc). The Visibility values here all mean differently levels of "visible", so it would seem weird to me to have one of them be "0". > this._inFullscreen and this._inOverview seem to be redundant with > this.visibility; can we just use this.visibility and pass the new mode into > _updateVisibility() ? No, because when leaving the overview, you need to know whether to go to Visibility.FULLSCREEN or Visibility.FULL, and you don't want to have to re-examine the window stacking. So you need this._inFullscreen. (We could get rid of this._inOverview, but that would make the code for the two cases less parallel.) (In reply to comment #17) > Not sure it's such a crime, but 'git bz apply' complained that there is a blank > line at EOF. fixed
pushing the accepted parts Attachment 159888 [details] pushed as e9e2786 - [Params] always return a new object from Params.parse Attachment 159889 [details] pushed as 1816a63 - [ShellGenericContainer] add _get_skip_paint() Attachment 159890 [details] pushed as 31914ab - [Chrome] redo using ShellGenericContainer Attachment 159891 [details] pushed as dd0882a - [Chrome] add "visibleInFullscreen" property Attachment 159915 [details] pushed as b71afe5 - [GnomeSession] split out the gnome-session presence D-Bus interface
(In reply to comment #23) > (In reply to comment #15) > > > this._inFullscreen and this._inOverview seem to be redundant with > > this.visibility; can we just use this.visibility and pass the new mode into > > _updateVisibility() ? > > No, because when leaving the overview, you need to know whether to go to > Visibility.FULLSCREEN or Visibility.FULL, and you don't want to have to > re-examine the window stacking. So you need this._inFullscreen. (We could get > rid of this._inOverview, but that would make the code for the two cases less > parallel.) So it seems that the code flow would still improve if we only use this._desktopInFullscreen to preserve the flag indicating that we should return to full screen and treat all the cases in parallel other than that. Otherwise, the reason for the usage of this._inFullscreen is non-obvious, because it seems to imply that we are in full screen when it is set to true, while it is actually supposed to stay set to true while we are in the overview. Another alternative might be to add another value to the enumeration, such as Visibility.OVERVIEW_FROM_FULLSCREEN, though it is probably wrong to overload the visibility state with the desktop state like that.
Can we close this now?
it looks like another patch got committed to deal with 'only urgent notifications when busy', so yes, there's nothing left here
Comment on attachment 159914 [details] [review] [MessageTray] make this visible in fullscreen mode very belatedly committing uncommitted patch...
*** Bug 652341 has been marked as a duplicate of this bug. ***