New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Global full screen mode blocks workspace switches #2974

Open
Streetwalrus opened this Issue Sep 21, 2017 · 14 comments

Comments

Projects
None yet
6 participants
@Streetwalrus
Contributor

Streetwalrus commented Sep 21, 2017

Output of i3 --moreversion 2>&- || i3 --version:

Binary i3 version:  4.14-100-gf34bb2b0 (2017-09-21, branch "next") © 2009 Michael Stapelberg and contributors
Running i3 version: 4.14-100-gf34bb2b0 (2017-09-21, branch "next") (pid 9333)
Loaded i3 config: /home/streetwalrus/.i3/config (Last modified: Thu 21 Sep 2017 20:41:08 IDT, 7580 seconds ago)

The i3 binary you just called: /home/streetwalrus/source/i3/build/i3
The i3 binary you are running: i3

This is commit 919e663 with #2849.

URL to a logfile as per http://i3wm.org/docs/debugging.html:

Not applicable.

What I did:

  1. Enable global full screen mode
  2. Try to switch to another workspace

What I saw:

Nothing happens.

What I expected instead:

The container should be hidden, and workspaces should switch, just like with regular full screen mode.

Checking the source tells me this is currently intended behavior, and removing the guards does allow switching workspaces but does not hide the full screen container.

A good way to handle this would be to return the container to regular full-screen mode when switching workspaces, and restore global full-screen when it gains focus again. Alternatively, only allow switching to a workspace on the same output as the one that container belongs to, and hide it when that happens.

Is such a behavior desirable, or should I focus on implementing this via IPC scripting instead?

@i3bot

This comment has been minimized.

Show comment
Hide comment
@i3bot

i3bot Sep 21, 2017

I don’t see a link to logs.i3wm.org. Did you follow http://i3wm.org/docs/debugging.html? (In case you actually provided a link to a logfile, please ignore me.)

i3bot commented Sep 21, 2017

I don’t see a link to logs.i3wm.org. Did you follow http://i3wm.org/docs/debugging.html? (In case you actually provided a link to a logfile, please ignore me.)

@Airblader

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Sep 21, 2017

Member

The current behavior strikes me as meaningful and mentally much easier to understand than your suggestion. I'd prefer to keep this behavior.

Member

Airblader commented Sep 21, 2017

The current behavior strikes me as meaningful and mentally much easier to understand than your suggestion. I'd prefer to keep this behavior.

@Streetwalrus

This comment has been minimized.

Show comment
Hide comment
@Streetwalrus

Streetwalrus Sep 21, 2017

Contributor

Here's a very crude patch that enables what I'm looking for, this kicks GFS windows into output FS mode when switching workspaces. The rest can be scripted easily.

diff --git a/src/commands.c b/src/commands.c
index 2697d6e1..597dde4f 100644
--- a/src/commands.c
+++ b/src/commands.c
@@ -847,12 +847,6 @@ void cmd_workspace(I3_CMD, const char *which) {
 
     DLOG("which=%s\n", which);
 
-    if (con_get_fullscreen_con(croot, CF_GLOBAL)) {
-        LOG("Cannot switch workspace while in global fullscreen\n");
-        ysuccess(false);
-        return;
-    }
-
     if (strcmp(which, "next") == 0)
         ws = workspace_next();
     else if (strcmp(which, "prev") == 0)
@@ -882,12 +876,6 @@ void cmd_workspace_number(I3_CMD, const char *which, const char *_no_auto_back_a
     const bool no_auto_back_and_forth = (_no_auto_back_and_forth != NULL);
     Con *output, *workspace = NULL;
 
-    if (con_get_fullscreen_con(croot, CF_GLOBAL)) {
-        LOG("Cannot switch workspace while in global fullscreen\n");
-        ysuccess(false);
-        return;
-    }
-
     long parsed_num = ws_name_to_number(which);
 
     if (parsed_num == -1) {
@@ -923,12 +911,6 @@ void cmd_workspace_number(I3_CMD, const char *which, const char *_no_auto_back_a
  *
  */
 void cmd_workspace_back_and_forth(I3_CMD) {
-    if (con_get_fullscreen_con(croot, CF_GLOBAL)) {
-        LOG("Cannot switch workspace while in global fullscreen\n");
-        ysuccess(false);
-        return;
-    }
-
     workspace_back_and_forth();
 
     cmd_output->needs_tree_render = true;
@@ -949,12 +931,6 @@ void cmd_workspace_name(I3_CMD, const char *name, const char *_no_auto_back_and_
         return;
     }
 
-    if (con_get_fullscreen_con(croot, CF_GLOBAL)) {
-        LOG("Cannot switch workspace while in global fullscreen\n");
-        ysuccess(false);
-        return;
-    }
-
     DLOG("should switch to workspace %s\n", name);
     if (!no_auto_back_and_forth && maybe_back_and_forth(cmd_output, name)) {
         ysuccess(true);
diff --git a/src/workspace.c b/src/workspace.c
index 4b350b82..3d44b792 100644
--- a/src/workspace.c
+++ b/src/workspace.c
@@ -480,6 +480,10 @@ static void _workspace_show(Con *workspace) {
 
     /* Push any sticky windows to the now visible workspace. */
     output_push_sticky_windows(old_focus);
+
+    /* Kick the previously focused window out of global fullscreen */
+    if (old_focus->fullscreen_mode == CF_GLOBAL)
+        con_enable_fullscreen(old_focus, CF_OUTPUT);
 }
 
 /*

I think adding this (behind an option like gfs_workspace_switch prevent|disable|output) should be reasonable enough.

Contributor

Streetwalrus commented Sep 21, 2017

Here's a very crude patch that enables what I'm looking for, this kicks GFS windows into output FS mode when switching workspaces. The rest can be scripted easily.

diff --git a/src/commands.c b/src/commands.c
index 2697d6e1..597dde4f 100644
--- a/src/commands.c
+++ b/src/commands.c
@@ -847,12 +847,6 @@ void cmd_workspace(I3_CMD, const char *which) {
 
     DLOG("which=%s\n", which);
 
-    if (con_get_fullscreen_con(croot, CF_GLOBAL)) {
-        LOG("Cannot switch workspace while in global fullscreen\n");
-        ysuccess(false);
-        return;
-    }
-
     if (strcmp(which, "next") == 0)
         ws = workspace_next();
     else if (strcmp(which, "prev") == 0)
@@ -882,12 +876,6 @@ void cmd_workspace_number(I3_CMD, const char *which, const char *_no_auto_back_a
     const bool no_auto_back_and_forth = (_no_auto_back_and_forth != NULL);
     Con *output, *workspace = NULL;
 
-    if (con_get_fullscreen_con(croot, CF_GLOBAL)) {
-        LOG("Cannot switch workspace while in global fullscreen\n");
-        ysuccess(false);
-        return;
-    }
-
     long parsed_num = ws_name_to_number(which);
 
     if (parsed_num == -1) {
@@ -923,12 +911,6 @@ void cmd_workspace_number(I3_CMD, const char *which, const char *_no_auto_back_a
  *
  */
 void cmd_workspace_back_and_forth(I3_CMD) {
-    if (con_get_fullscreen_con(croot, CF_GLOBAL)) {
-        LOG("Cannot switch workspace while in global fullscreen\n");
-        ysuccess(false);
-        return;
-    }
-
     workspace_back_and_forth();
 
     cmd_output->needs_tree_render = true;
@@ -949,12 +931,6 @@ void cmd_workspace_name(I3_CMD, const char *name, const char *_no_auto_back_and_
         return;
     }
 
-    if (con_get_fullscreen_con(croot, CF_GLOBAL)) {
-        LOG("Cannot switch workspace while in global fullscreen\n");
-        ysuccess(false);
-        return;
-    }
-
     DLOG("should switch to workspace %s\n", name);
     if (!no_auto_back_and_forth && maybe_back_and_forth(cmd_output, name)) {
         ysuccess(true);
diff --git a/src/workspace.c b/src/workspace.c
index 4b350b82..3d44b792 100644
--- a/src/workspace.c
+++ b/src/workspace.c
@@ -480,6 +480,10 @@ static void _workspace_show(Con *workspace) {
 
     /* Push any sticky windows to the now visible workspace. */
     output_push_sticky_windows(old_focus);
+
+    /* Kick the previously focused window out of global fullscreen */
+    if (old_focus->fullscreen_mode == CF_GLOBAL)
+        con_enable_fullscreen(old_focus, CF_OUTPUT);
 }
 
 /*

I think adding this (behind an option like gfs_workspace_switch prevent|disable|output) should be reasonable enough.

@Airblader

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Sep 22, 2017

Member

I think adding this (behind an option like gfs_workspace_switch prevent|disable|output) should be reasonable enough.

I don't think this is worth a configuration directive, see https://faq.i3wm.org/question/778/why-is-patch-not-merged-and-made-optional/

I still maintain that the current behavior is meaningful and simple to understand. I currently tend towards closing this. @stapelberg What do you think?

Member

Airblader commented Sep 22, 2017

I think adding this (behind an option like gfs_workspace_switch prevent|disable|output) should be reasonable enough.

I don't think this is worth a configuration directive, see https://faq.i3wm.org/question/778/why-is-patch-not-merged-and-made-optional/

I still maintain that the current behavior is meaningful and simple to understand. I currently tend towards closing this. @stapelberg What do you think?

@SijmenSchoon

This comment has been minimized.

Show comment
Hide comment
@SijmenSchoon

SijmenSchoon Sep 22, 2017

To me, @Streetwalrus' suggestion sounds more than reasonable. Having nothing happen when pressing the switch combination feels counter-intuitive, and at least having the option of kicking the application out of full screen would be useful.

SijmenSchoon commented Sep 22, 2017

To me, @Streetwalrus' suggestion sounds more than reasonable. Having nothing happen when pressing the switch combination feels counter-intuitive, and at least having the option of kicking the application out of full screen would be useful.

@Airblader

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Sep 22, 2017

Member

You have that option, it's called fullscreen disable. Given that global fullscreen only happens if you set it yourself you should also never come in the situation where you're not really aware of the layout.

I definitely don't want to make this configurable because this isn't worth the added configuration complexity. The only question would be whether to make this change for all users or not.

This behavior can also be entirely scripted.

Member

Airblader commented Sep 22, 2017

You have that option, it's called fullscreen disable. Given that global fullscreen only happens if you set it yourself you should also never come in the situation where you're not really aware of the layout.

I definitely don't want to make this configurable because this isn't worth the added configuration complexity. The only question would be whether to make this change for all users or not.

This behavior can also be entirely scripted.

@Streetwalrus

This comment has been minimized.

Show comment
Hide comment
@Streetwalrus

Streetwalrus Sep 22, 2017

Contributor

For what it's worth, a lot of things that do not actually make sense while global fullscreen is enabled are allowed, such as moving containers and workspaces around. These should be blocked.

Contributor

Streetwalrus commented Sep 22, 2017

For what it's worth, a lot of things that do not actually make sense while global fullscreen is enabled are allowed, such as moving containers and workspaces around. These should be blocked.

@Airblader

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Sep 22, 2017

Member

You mean via match criteria or requests? That seems like a separate topic to me, I don't really see a resemblance to the workspace switching case.

However, this did get me thinking. A case that actually is similar is focusing a container on the workspace, which is an open issue and where we decided that this should result in fullscreen being disabled. I guess the consistency argument would apply in this case (since workspace switching is in many way the same as changing focus), see #1819.

To be clear, this would mean that the consistent behavior would be to disable fullscreen entirely (not make it output-fullscreened and also no automatic re-fullscreening).

@stapelberg Perhaps we should actually change the behavior to drop out of fullscreen if the workspace is switched, what do you think?

Member

Airblader commented Sep 22, 2017

You mean via match criteria or requests? That seems like a separate topic to me, I don't really see a resemblance to the workspace switching case.

However, this did get me thinking. A case that actually is similar is focusing a container on the workspace, which is an open issue and where we decided that this should result in fullscreen being disabled. I guess the consistency argument would apply in this case (since workspace switching is in many way the same as changing focus), see #1819.

To be clear, this would mean that the consistent behavior would be to disable fullscreen entirely (not make it output-fullscreened and also no automatic re-fullscreening).

@stapelberg Perhaps we should actually change the behavior to drop out of fullscreen if the workspace is switched, what do you think?

@Streetwalrus

This comment has been minimized.

Show comment
Hide comment
@Streetwalrus

Streetwalrus Sep 22, 2017

Contributor

You mean via match criteria or requests? That seems like a separate topic to me, I don't really see a resemblance to the workspace switching case.

Nope, just regular bindings.

To be clear, this would mean that the consistent behavior would be to disable fullscreen entirely (not make it output-fullscreened and also no automatic re-fullscreening).

I would be fine with this.

Contributor

Streetwalrus commented Sep 22, 2017

You mean via match criteria or requests? That seems like a separate topic to me, I don't really see a resemblance to the workspace switching case.

Nope, just regular bindings.

To be clear, this would mean that the consistent behavior would be to disable fullscreen entirely (not make it output-fullscreened and also no automatic re-fullscreening).

I would be fine with this.

@Airblader

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Sep 22, 2017

Member

Nope, just regular bindings.

How do you use regular bindings which don't use match criteria to move a non-focused window?

Member

Airblader commented Sep 22, 2017

Nope, just regular bindings.

How do you use regular bindings which don't use match criteria to move a non-focused window?

@Streetwalrus

This comment has been minimized.

Show comment
Hide comment
@Streetwalrus

Streetwalrus Sep 22, 2017

Contributor

It moves the focused window or workspace, but that happens without any kind of feedback since we're in GFS mode the whole time.

Contributor

Streetwalrus commented Sep 22, 2017

It moves the focused window or workspace, but that happens without any kind of feedback since we're in GFS mode the whole time.

@Airblader

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Sep 22, 2017

Member

You mean the focused window of a workspace which doesn't contain the globally fullscreened window? Because (at least) on its workspace it should be the focused window. In any case, yes, this sounds like a bug for sure.

Member

Airblader commented Sep 22, 2017

You mean the focused window of a workspace which doesn't contain the globally fullscreened window? Because (at least) on its workspace it should be the focused window. In any case, yes, this sounds like a bug for sure.

@Streetwalrus

This comment has been minimized.

Show comment
Hide comment
@Streetwalrus

Streetwalrus Sep 22, 2017

Contributor

The globally fullscreened window is the focused one, yes.

Contributor

Streetwalrus commented Sep 22, 2017

The globally fullscreened window is the focused one, yes.

@stapelberg

This comment has been minimized.

Show comment
Hide comment
@stapelberg

stapelberg Sep 23, 2017

Member

@stapelberg Perhaps we should actually change the behavior to drop out of fullscreen if the workspace is switched, what do you think?

That seems reasonable to me.

Users who prefer to keep their window in fullscreen mode could resort to entering a binding mode when entering fullscreen, and have that mode only contain commands to exit fullscreen.

Member

stapelberg commented Sep 23, 2017

@stapelberg Perhaps we should actually change the behavior to drop out of fullscreen if the workspace is switched, what do you think?

That seems reasonable to me.

Users who prefer to keep their window in fullscreen mode could resort to entering a binding mode when entering fullscreen, and have that mode only contain commands to exit fullscreen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment