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 654550 - Message tray visible when screensaver active
Message tray visible when screensaver active
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: message-tray
3.1.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-07-13 12:45 UTC by Cosimo Cecchi
Modified: 2011-07-14 16:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
chrome: don't show visibleInOverview chrome when the screensaver is active (5.69 KB, patch)
2011-07-13 14:39 UTC, Dan Winship
needs-work Details | Review
statusMenu: fix typo (950 bytes, patch)
2011-07-13 19:14 UTC, Dan Winship
accepted-commit_now Details | Review
screenSaver: bugfixes (3.04 KB, patch)
2011-07-13 19:15 UTC, Dan Winship
committed Details | Review
chrome: don't show visibleInOverview chrome when the screensaver is active (2.05 KB, patch)
2011-07-13 19:15 UTC, Dan Winship
committed Details | Review

Description Cosimo Cecchi 2011-07-13 12:45:09 UTC
Recently the message tray can be made visible also when an application is in fullscreen mode; I believe this should be disabled when the screensaver is active. Right now (using shell 3.1.2), it's always visible, notifications can be read and activated, and status icons receive events, even though no window opened this way jumps on top of the screensaver itself.
Comment 1 Dan Winship 2011-07-13 14:39:54 UTC
Created attachment 191891 [details] [review]
chrome: don't show visibleInOverview chrome when the screensaver is active

visibleInOverview chrome was visible even when the screensaver was
active. Although we may eventually need visibleInScreenSaver, that
should be a separate flag.

Fix this by tracking the screensaver active state, and hiding the chrome
when the screensaver is active.

=====

I actually already had this patch lying around (the bug is much more
noticeable with the on-screen keyboard running...)
Comment 2 Florian Müllner 2011-07-13 14:51:10 UTC
Review of attachment 191891 [details] [review]:

Makes sense to me, but the new screenSaver prototype conflicts with a patch in bug 653520 which hopefully will land today, so marking needs-work to get it off the list.
Comment 3 Dan Winship 2011-07-13 19:14:56 UTC
Created attachment 191916 [details] [review]
statusMenu: fix typo
Comment 4 Dan Winship 2011-07-13 19:15:06 UTC
Created attachment 191917 [details] [review]
screenSaver: bugfixes

Fix the signal handling; you can't use this.connect('ActiveChanged')
to connect to a D-Bus signal after replacing the signal methods with
the lang.signals versions. Just leave it using the D-Bus signal names,
just like it uses the D-Bus method names.

Also, remove the "_" from "_screenSaverActive", to match what
AutomountManager checks for, and remove getActive(), since it's not
needed.
Comment 5 Dan Winship 2011-07-13 19:15:15 UTC
Created attachment 191918 [details] [review]
chrome: don't show visibleInOverview chrome when the screensaver is active

visibleInOverview chrome was visible even when the screensaver was
active. Although we may eventually need visibleInScreenSaver, that
should be a separate flag.

Fix this by tracking the screensaver active state, and hiding the chrome
when the screensaver is active.
Comment 6 Florian Müllner 2011-07-13 20:50:11 UTC
Review of attachment 191918 [details] [review]:

Looks good, except for the callback to GetActiveRemote.

::: js/ui/chrome.js
@@ +56,3 @@
+            function(result, err) {
+                if (result)
+                    this._onScreenSaverActiveChanged(this._screenSaverProxy, result[0]);

I don't think we still stuff the return value into an array; quick test:

  log(result); // => false
  log(result.length) // => undefined
Comment 7 Florian Müllner 2011-07-13 20:50:18 UTC
Review of attachment 191916 [details] [review]:

Gah. Sorry for not catching that ...
Comment 8 Florian Müllner 2011-07-13 20:50:22 UTC
Review of attachment 191917 [details] [review]:

Oh, didn't know about the signal methods. Looks good!
Comment 9 Florian Müllner 2011-07-13 21:56:50 UTC
Comment on attachment 191916 [details] [review]
statusMenu: fix typo

Fixed by commit fe82897064f0.
Comment 10 Dan Winship 2011-07-14 16:51:33 UTC
Attachment 191917 [details] pushed as 5f86e29 - screenSaver: bugfixes
Attachment 191918 [details] pushed as 896d8e8 - chrome: don't show visibleInOverview chrome when the screensaver is active