GNOME Bugzilla – Bug 701717
Classic: Windows sometimes change workspace after screen lock
Last modified: 2013-06-07 22:57:36 UTC
GNOME Classic includes an extension that fixes the number of available workspaces in the system. However, in certain (easily reproducible) circumstances, windows on these workspaces will be relocated when locking and unlocking the screen. Steps to reproduce: 1) Install and log into gnome-classic-session 2) Create an application window on the first workspace (doesn't matter what application) 3) Create a second application window and position it on the *third* workspace 4) Ensure that there are zero windows in the second workspace (between the two workspaces with windows). 5) Lock the screen and unlock it again Actual result: The window that was previously on the third workspace is now on the second workspace and the third workspace is empty. Expected result: Windows should remain where they were, prior to locking. Additional information: I have a workflow that involves four workspaces: 1) Email 2) Web Browser 3) Code Editor 4) IRC I do not always have anything in use on the "Code Editor" workspace. I have trained muscle-memory that tells me how to get to my IRC window by hitting ctrl-alt-down "a whole bunch of times" because I know my IRC window is always on the last accessible workspace. The fact that it has been moved when I lock the screen throws off my workflow and forces me to backtrack. (Yes, I am aware that this would also not work for me if I was using the standard gnome-shell with dynamic workspaces).
It's clear what's happening: (1) classic-specific default settings are implemented as an extension (2) extensions are disabled while the screen is locked => workspaces are dynamic during lock, so all empty workspaces but the last one are removed Now, it's far less clear how to fix this: - we can allow some extensions to persist while locked (user-themes users will love this, but I'd rather not go there) - we can use some DCONF_PROFILE hack in gnome-session instead of an extension (someone wanted to take a look into this a while ago, but it looks like it didn't go anywhere) - we can make GSettings overrides a mode-specific property (off-hand my preferred option, but tweak-tool will need tweaking)
Personally, I agree that the third option is the best one. That way we can also remove those extensions, they are quite confusing when the same settings are available from the tweak tool.
(In reply to comment #2) > That way we can also remove those extensions, they are quite confusing when the > same settings are available from the tweak tool. Yeah, testing this right now ...
Created attachment 246170 [details] [review] main: Move pref overrides to JS We will allow to use mode-specific overrides; in preparation for that, move the code so that we only override preferences after initializing the session mode.
Created attachment 246171 [details] [review] main: Pick up overridesSchema from sessionMode This will allow the use of mode-specific defaults. For classic mode we currently implement this with mini-extensions, but this may result in confusing behavior when settings change due to extensions being disabled during screen locks (not to mention that those mini-extensions are hardly an elegant approach).
Created attachment 246172 [details] [review] classic: Replace mini-extensions with mode-specific overridesSchema Some default values differ between classic and normal sessions. We used to implement this by overriding the shell's default values in mini-extensions (or more precisely: reversing the shell's overrides). Use a mode-specific overridesSchema instead, which has the advantage that settings defaults will not change unexpectedly when extensions are disabled/enabled (for instance during screen locks).
Review of attachment 246171 [details] [review]: ::: js/ui/main.js @@ +124,3 @@ + overridesSchema = sessionMode.overridesSchema; + else + overridesSchema = OVERRIDES_SCHEMA; Shouldn't this be in the default restricted session mode?
(In reply to comment #7) > Shouldn't this be in the default restricted session mode? Yeah, it is - I just wrote that part of the patch before actually adding it to sessionMode :-)
Review of attachment 246172 [details] [review]: ::: data/Makefile.am @@ +30,3 @@ +gsettings_in_files = org.gnome.shell.extensions.classic-overrides.gschema.xml.in +gsettings_SCHEMAS = $(gsettings_in_files:.xml.in=.xml) You don't need the .in file, intltool handles GSettings directly.
Review of attachment 246170 [details] [review]: Nice.
Created attachment 246185 [details] [review] main: Pick up overridesSchema from sessionMode Updated according to review.
Created attachment 246186 [details] [review] windowManager: Use the correct schema for 'dynamic-workspaces' We currently monitor the shell's override schema for changes to the 'dynamic-workspaces' key, which ends up being the wrong schema in classic mode. With the new ability to use mode-specific overides, we can finally fix this.
Created attachment 246187 [details] [review] classic: Replace mini-extensions with mode-specific overridesSchema (In reply to comment #9) > You don't need the .in file, intltool handles GSettings directly. Not sure what you mean here, this is in line with settings.mk (also I get a distcheck breakage when using .xml directly) => unchanged. I did add the schema to POTFILES.in though, which I missed before ...
Review of attachment 246185 [details] [review]: Just a style nit. ::: js/ui/main.js @@ +122,2 @@ for (let i = 0; i < keys.length; i++) + Meta.prefs_override_preference_schema (keys[i], sessionMode.overridesSchema); No space before (
Review of attachment 246186 [details] [review]: Yes
(In reply to comment #13) > Created an attachment (id=246187) [details] [review] > classic: Replace mini-extensions with mode-specific overridesSchema > > (In reply to comment #9) > > You don't need the .in file, intltool handles GSettings directly. > > Not sure what you mean here, this is in line with settings.mk (also I get a > distcheck breakage when using .xml directly) => unchanged. > I did add the schema to POTFILES.in though, which I missed before ... Distcheck breakage? What error? What I meant anyway is that you don't need rules in the makefile to handle translations, you just add the .xml file to POTFILES.in, intltool notices .gschema.xml and does the right thing.
(In reply to comment #17) > Distcheck breakage? What error? builddir != srcdir. I guess it is fixable by throwing in the right amount of $(srcdir), $(top_srcdir) etc, but while the extensions themselves keep using .in files, consistency beats saving three lines in Makefile.am IMHO ...
Created attachment 246194 [details] [review] main: Use the correct schema for 'dynamic-workspaces' Same as attachment 246186 [details] [review], but for the gnome-3-8 branch
Review of attachment 246170 [details] [review]: You're not moving the OVERRIDES_SCHEMA definition.
Review of attachment 246185 [details] [review]: OK, disregard my previous comment :-)
Review of attachment 246194 [details] [review]: Looks good.
(In reply to comment #20) > You're not moving the OVERRIDES_SCHEMA definition. (In reply to comment #21) > OK, disregard my previous comment :-) But you should still remove the definition from the C code!
(In reply to comment #21) > Review of attachment 246185 [details] [review]: > > OK, disregard my previous comment :-) Well, the previous commit is not supposed to break (even temporarily) either. It's a cherry-pick issue, the constant does exist on the 3-8 branch. I'll fix this before pushing.
(In reply to comment #23) > But you should still remove the definition from the C code! True.
Attachment 246170 [details] pushed as 6c2f3d1 - main: Move pref overrides to JS Attachment 246185 [details] pushed as 91844e4 - main: Pick up overridesSchema from sessionMode Attachment 246186 [details] pushed as 5cd913a - windowManager: Use the correct schema for 'dynamic-workspaces'
Comment on attachment 246194 [details] [review] main: Use the correct schema for 'dynamic-workspaces' Attachment 246194 [details] pushed as 3ff1942 - main: Use the correct schema for 'dynamic-workspaces'
Attachment 246187 [details] pushed as 0ffaf62 - classic: Replace mini-extensions with mode-specific overridesSchema Pushing after approval on IRC.