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 749383 - window-list: after unlocking computer, even if active app is fullscreen, window-list is showing
window-list: after unlocking computer, even if active app is fullscreen, wind...
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: extensions
3.14.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2015-05-14 14:49 UTC by jleb.git
Modified: 2015-05-21 12:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
layout: Set initial visibility of fullscreen-tracking chrome (1.12 KB, patch)
2015-05-14 16:26 UTC, Florian Müllner
none Details | Review
layout: Set initial visibility of fullscreen-tracking chrome (2.42 KB, patch)
2015-05-21 11:34 UTC, Florian Müllner
committed Details | Review

Description jleb.git 2015-05-14 14:49:32 UTC
Problem:

The window-list extension is showing even if the app is fullscreen if it's the active window after successfully unlocking the computer.

Steps to reproduce:

1. Fullscreen any app (e.g. Firefox).
2. Lock the computer (e.g. Super+L).
3. Unlock the computer.

Expected Result:

Firefox is fullscreen with no window-list at the bottom.

Actual Result:

The window-list is showing.
Comment 1 jleb.git 2015-05-14 14:51:06 UTC
Environment:

F21, 3.19.5-200

$ rpm -qa | grep gnome-shell
gnome-shell-extension-background-logo-3.14.0-2.fc21.noarch
gnome-shell-extension-launch-new-instance-3.14.4-1.fc21.noarch
gnome-shell-extension-common-3.14.4-1.fc21.noarch
gnome-shell-extension-alternate-tab-3.14.4-1.fc21.noarch
gnome-shell-extension-user-theme-3.14.4-1.fc21.noarch
gnome-shell-3.14.4-2.fc21.x86_64
gnome-shell-extension-places-menu-3.14.4-1.fc21.noarch
gnome-shell-extension-apps-menu-3.14.4-1.fc21.noarch
gnome-shell-extension-window-list-3.14.4-1.fc21.noarch
Comment 2 Florian Müllner 2015-05-14 16:26:04 UTC
Created attachment 303382 [details] [review]
layout: Set initial visibility of fullscreen-tracking chrome

When chrome is added with the trackFullscreen parameter, the actor's
visibility will be updated automatically whenever its monitor's
fullscreen state changes. However as we currently ignore the fullscreen
state at the time the chrome is added, the initial visibility may well
be incorrect - fix this by updating the initial visibility as necessary.
Comment 3 jleb.git 2015-05-14 18:11:57 UTC
(In reply to Florian Müllner from comment #2)
> Created attachment 303382 [details] [review] [review]
> layout: Set initial visibility of fullscreen-tracking chrome

Thanks for the quick patch! Do I need to rebuild the extension to use this, or is there a layout.js file I can apply it to?
Comment 4 Florian Müllner 2015-05-14 19:41:58 UTC
(In reply to jleb.git from comment #3)
> (In reply to Florian Müllner from comment #2)
> Do I need to rebuild the extension to use this,
> or is there a layout.js file I can apply it to?

The fix is for gnome-shell itself, not the extension. As the javascript sources are now compiled into the gnome-shell executable, the file is not readily available. However you can still test the patch if you follow the instructions in https://blogs.gnome.org/mclasen/2014/03/24/keeping-gnome-shell-approachable/.
Comment 5 Rui Matos 2015-05-15 12:21:16 UTC
Review of attachment 303382 [details] [review]:

makes sense
Comment 6 Rui Matos 2015-05-15 12:28:14 UTC
Review of attachment 303382 [details] [review]:

::: js/ui/layout.js
@@ +839,3 @@
         this._trackedActors.push(actorData);
+        if (actorData.trackFullscreen && this.monitors.length)
+            this._updateVisibility();

Wait. This will iterate all this._trackedActors list for each actor we add.

Perhaps factor out a method to update the visibility of a single tracked actor out of _updateVisibility() and use both here and there?
Comment 7 Florian Müllner 2015-05-21 11:34:36 UTC
Created attachment 303750 [details] [review]
layout: Set initial visibility of fullscreen-tracking chrome

(In reply to Rui Matos from comment #6)
> Wait. This will iterate all this._trackedActors list for each actor we add.

Yeah, I originally just used the monitor.inFullscreen check which wasn't quite accurate, and then got lazy - it's not that bad actually, considering that the iteration is only done for actors that track fullscreen (and only after monitors are initialized), so gnome-shell proper doesn't hit this code path at all. But that's not a good excuse for being lazy, so yeah, let's refactor this a bit ...
Comment 8 Rui Matos 2015-05-21 11:50:19 UTC
Review of attachment 303750 [details] [review]:

all good
Comment 9 Florian Müllner 2015-05-21 12:00:59 UTC
Attachment 303750 [details] pushed as dc4b8c8 - layout: Set initial visibility of fullscreen-tracking chrome