Refined the fullscreen focus constraints.

Basically, a focus change can't escape a fullscreen container. The
only exception is per-output fullscreen containers, as you should
be able to focus a container in a different workspace in this case.

This is an improvement on 4eab046e, now considering the difference
between global and per-output fullscreen and taking the tree
structure into account to determine what escaping the fullscreen
container means. It only affects targeted focus commands in the
form "for_window [...] focus", but it lays the foundation for
forthcoming fixes to all other focus commands.

diff --git a/include/con.h b/include/con.h
index b14c477e53de6fb864b8d1dc06e29c1b36510fd3..95726147a74d56a053141c847fc0f5b3fb7d6843 100644 (file)
--- a/include/con.h
+++ b/include/con.h
@@ -255,4 +255,27 @@ void con_set_layout(Con *con, int layout);
  */
 Rect con_minimum_size(Con *con);
 
+/**
+ * Returns true if changing the focus to con would be allowed considering
+ * the fullscreen focus constraints. Specifically, if a fullscreen container or
+ * any of its descendants is focused, this function returns true if and only if
+ * focusing con would mean that focus would still be visible on screen, i.e.,
+ * the newly focused container would not be obscured by a fullscreen container.
+ *
+ * In the simplest case, if a fullscreen container or any of its descendants is
+ * fullscreen, this functions returns true if con is the fullscreen container
+ * itself or any of its descendants, as this means focus wouldn't escape the
+ * boundaries of the fullscreen container.
+ *
+ * In case the fullscreen container is of type CF_OUTPUT, this function returns
+ * true if con is on a different workspace, as focus wouldn't be obscured by
+ * the fullscreen container that is constrained to a different workspace.
+ *
+ * Note that this same logic can be applied to moving containers. If a
+ * container can be focused under the fullscreen focus constraints, it can also
+ * become a parent or sibling to the currently focused container.
+ *
+ */
+bool con_fullscreen_permits_focusing(Con *con);
+
 #endif

diff --git a/src/commands.c b/src/commands.c
index 1e1ee0ff243190aafa4d9f2a8e34256702618a4c..b3c955e56d926e3b2fe8eab3e795f7c0d534356e 100644 (file)
--- a/src/commands.c
+++ b/src/commands.c
@@ -1262,13 +1262,9 @@ void cmd_focus(I3_CMD) {
         if (!ws)
             continue;
 
-        /* Don't allow the focus switch if the focused and current
-         * containers are in the same workspace. */
-        if (focused &&
-            focused->type != CT_WORKSPACE &&
-            focused->fullscreen_mode != CF_NONE &&
-            con_get_workspace(focused) == ws) {
-            LOG("Cannot change focus while in fullscreen mode (same workspace).\n");
+        /* Check the fullscreen focus constraints. */
+        if (!con_fullscreen_permits_focusing(current->con)) {
+            LOG("Cannot change focus while in fullscreen mode (fullscreen rules).\n");
             ysuccess(false);
             return;
         }

diff --git a/src/con.c b/src/con.c
index c24a379f7abc186a1fd2dc5944da7a1e03992be0..f969d0569c791f211d35523a7271188c838b6494 100644 (file)
--- a/src/con.c
+++ b/src/con.c
@@ -1132,3 +1132,59 @@ Rect con_minimum_size(Con *con) {
          con->type, con->layout, con->orientation);
     assert(false);
 }
+
+/*
+ * Returns true if changing the focus to con would be allowed considering
+ * the fullscreen focus constraints. Specifically, if a fullscreen container or
+ * any of its descendants is focused, this function returns true if and only if
+ * focusing con would mean that focus would still be visible on screen, i.e.,
+ * the newly focused container would not be obscured by a fullscreen container.
+ *
+ * In the simplest case, if a fullscreen container or any of its descendants is
+ * fullscreen, this functions returns true if con is the fullscreen container
+ * itself or any of its descendants, as this means focus wouldn't escape the
+ * boundaries of the fullscreen container.
+ *
+ * In case the fullscreen container is of type CF_OUTPUT, this function returns
+ * true if con is on a different workspace, as focus wouldn't be obscured by
+ * the fullscreen container that is constrained to a different workspace.
+ *
+ * Note that this same logic can be applied to moving containers. If a
+ * container can be focused under the fullscreen focus constraints, it can also
+ * become a parent or sibling to the currently focused container.
+ *
+ */
+bool con_fullscreen_permits_focusing(Con *con) {
+    /* No focus, no problem. */
+    if (!focused)
+        return true;
+
+    /* Find the first fullscreen ascendent. */
+    Con *fs = focused;
+    while (fs && fs->fullscreen_mode == CF_NONE)
+        fs = fs->parent;
+
+    /* The most common case is we hit the workspace level. In this
+     * situation, changing focus is also harmless. */
+    assert(fs->fullscreen_mode != CF_NONE);
+    if (fs->type == CT_WORKSPACE)
+        return true;
+
+    /* If fullscreen is per-output, the focus being in a different workspace is
+     * sufficient to guarantee that change won't leave fullscreen in bad shape. */
+    if (fs->fullscreen_mode == CF_OUTPUT &&
+        con_get_workspace(con) != con_get_workspace(fs)) {
+            return true;
+    }
+
+    /* Allow it only if the container to be focused is contained within the
+     * current fullscreen container. */
+    do {
+        if (con->parent == fs)
+            return true;
+        con = con->parent;
+    } while (con);
+
+    /* Focusing con would hide it behind a fullscreen window, disallow it. */
+    return false;
+}

diff --git a/testcases/t/156-fullscreen-focus.t b/testcases/t/156-fullscreen-focus.t
index f9dc6dce1fe48e1fe158b10c629578fc5e6b780e..3a27c9ffa28ea36d50dd35f4f467937f0e6143c6 100644 (file)
--- a/testcases/t/156-fullscreen-focus.t
+++ b/testcases/t/156-fullscreen-focus.t
@@ -11,9 +11,9 @@ my $i3 = i3(get_socket_path());
 
 my $tmp = fresh_workspace;
 
-#####################################################################
-# open the left window
-#####################################################################
+################################################################################
+# Open the left window.
+################################################################################
 
 my $left = open_window({ background_color => '#ff0000' });
 
@@ -21,23 +21,26 @@ is($x->input_focus, $left->id, 'left window focused');
 
 diag("left = " . $left->id);
 
-#####################################################################
-# Open the right window
-#####################################################################
+################################################################################
+# Open the right window.
+################################################################################
 
 my $right = open_window({ background_color => '#00ff00' });
 
 diag("right = " . $right->id);
 
-#####################################################################
-# Set the right window to fullscreen
-#####################################################################
+################################################################################
+# Set the right window to fullscreen.
+################################################################################
+
 cmd 'nop setting fullscreen';
 cmd 'fullscreen';
 
-#####################################################################
-# Open a third window
-#####################################################################
+################################################################################
+# Open a third window. Since we're fullscreen, the window won't be # mapped, so
+# don't wait for it to be mapped. Instead, just send the map request and sync
+# with i3 to make sure i3 recognizes it.
+################################################################################
 
 my $third = open_window({
         background_color => '#0000ff',
@@ -51,13 +54,15 @@ sync_with_i3;
 
 diag("third = " . $third->id);
 
-# move the fullscreen window to a different ws
+################################################################################
+# Move the window to a different workspace, and verify that the third window now
+# gets focused in the current workspace.
+################################################################################
 
 my $tmp2 = get_unused_workspace;
 
 cmd "move workspace $tmp2";
 
-# verify that the third window has the focus
 is($x->input_focus, $third->id, 'third window focused');
 
 ################################################################################
@@ -87,20 +92,67 @@ is($nodes->[0]->{id}, $old_id, 'id unchanged');
 is($nodes->[0]->{focused}, 1, 'fullscreen window focused');
 
 ################################################################################
-# Make sure it's possible to focus a container in a different workspace even if
-# we are currently focusing a fullscreen container.
+# Ensure it's possible to change focus if it doesn't escape the fullscreen
+# container with fullscreen global. We can't even focus a container in a
+# different workspace.
 ################################################################################
 
-$tmp2 = fresh_workspace;
-my $focusable_window = open_window;
+cmd 'fullscreen';
 
+$tmp = fresh_workspace;
 cmd "workspace $tmp";
-cmd '[id="' . $focusable_window->id . '"] focus';
+my $diff_ws = open_window;
 
-is(focused_ws(), $tmp2, 'focus went to a different workspace');
+$tmp2 = fresh_workspace;
+cmd "workspace $tmp2";
+cmd 'split h';
 
-$nodes = get_ws_content($tmp2);
-is(scalar @$nodes, 1, 'precisely one window');
-is($nodes->[0]->{focused}, 1, 'focusable window focused');
+$left = open_window;
+my $right1 = open_window;
+cmd 'split v';
+my $right2 = open_window;
+$nodes = get_ws_content($tmp);
+
+cmd 'focus parent';
+cmd 'fullscreen global';
+
+cmd '[id="' . $right1->id . '"] focus';
+is($x->input_focus, $right1->id, 'upper right window focused');
+
+cmd '[id="' . $right2->id . '"] focus';
+is($x->input_focus, $right2->id, 'bottom right window focused');
+
+cmd '[id="' . $left->id . '"] focus';
+is($x->input_focus, $right2->id, 'prevented focus change to left window');
+
+cmd '[id="' . $diff_ws->id . '"] focus';
+is($x->input_focus, $right2->id, 'prevented focus change to different ws');
+
+################################################################################
+# Same tests when we're in non-global fullscreen mode. We toggle fullscreen on
+# and off to avoid testing whether focus level works in fullscreen for now. It
+# should now be possible to focus a container in a different workspace.
+################################################################################
+
+cmd 'fullscreen global';
+cmd 'fullscreen global';
+
+cmd '[id="' . $right1->id . '"] focus';
+is($x->input_focus, $right1->id, 'upper right window focused');
+
+cmd 'focus parent';
+cmd 'fullscreen';
+
+cmd '[id="' . $right1->id . '"] focus';
+is($x->input_focus, $right1->id, 'upper right window still focused');
+
+cmd '[id="' . $right2->id . '"] focus';
+is($x->input_focus, $right2->id, 'bottom right window focused');
+
+cmd '[id="' . $left->id . '"] focus';
+is($x->input_focus, $right2->id, 'prevented focus change to left window');
+
+cmd '[id="' . $diff_ws->id . '"] focus';
+is($x->input_focus, $diff_ws->id, 'allowed focus change to different ws');
 
 done_testing;