Merge pull request #3087 from orestisf1993/focus_order

Fix focus order issues when encapsulating workspaces

include/con.h

@@ -228,6 +228,22 @@ void con_unmark(Con *con, const char *name);
  */
 Con *con_for_window(Con *con, i3Window *window, Match **store_match);
 
+/**
+ * Iterate over the container's focus stack and return an array with the
+ * containers inside it, ordered from higher focus order to lowest.
+ *
+ */
+Con **get_focus_order(Con *con);
+
+/**
+ * Clear the container's focus stack and re-add it using the provided container
+ * array. The function doesn't check if the provided array contains the same
+ * containers with the previous focus stack but will not add floating containers
+ * in the new focus stack if container is not a workspace.
+ *
+ */
+void set_focus_order(Con *con, Con **focus_order);
+
 /**
  * Returns the number of children of this container.
  *

src/con.c

@@ -809,6 +809,62 @@ Con *con_for_window(Con *con, i3Window *window, Match **store_match) {
     return NULL;
 }
 
+static int num_focus_heads(Con *con) {
+    int focus_heads = 0;
+
+    Con *current;
+    TAILQ_FOREACH(current, &(con->focus_head), focused) {
+        focus_heads++;
+    }
+
+    return focus_heads;
+}
+
+/*
+ * Iterate over the container's focus stack and return an array with the
+ * containers inside it, ordered from higher focus order to lowest.
+ *
+ */
+Con **get_focus_order(Con *con) {
+    const int focus_heads = num_focus_heads(con);
+    Con **focus_order = smalloc(focus_heads * sizeof(Con *));
+    Con *current;
+    int idx = 0;
+    TAILQ_FOREACH(current, &(con->focus_head), focused) {
+        assert(idx < focus_heads);
+        focus_order[idx++] = current;
+    }
+
+    return focus_order;
+}
+
+/*
+ * Clear the container's focus stack and re-add it using the provided container
+ * array. The function doesn't check if the provided array contains the same
+ * containers with the previous focus stack but will not add floating containers
+ * in the new focus stack if container is not a workspace.
+ *
+ */
+void set_focus_order(Con *con, Con **focus_order) {
+    int focus_heads = 0;
+    while (!TAILQ_EMPTY(&(con->focus_head))) {
+        Con *current = TAILQ_FIRST(&(con->focus_head));
+
+        TAILQ_REMOVE(&(con->focus_head), current, focused);
+        focus_heads++;
+    }
+
+    for (int idx = 0; idx < focus_heads; idx++) {
+        /* Useful when encapsulating a workspace. */
+        if (con->type != CT_WORKSPACE && con_inside_floating(focus_order[idx])) {
+            focus_heads++;
+            continue;
+        }
+
+        TAILQ_INSERT_TAIL(&(con->focus_head), focus_order[idx], focused);
+    }
+}
+
 /*
  * Returns the number of children of this container.
  *
@@ -1781,17 +1837,9 @@ void con_set_layout(Con *con, layout_t layout) {
             new->layout = layout;
             new->last_split_layout = con->last_split_layout;
 
-            /* Save the container that was focused before we move containers
-             * around, but only if the container is visible (otherwise focus
-             * will be restored properly automatically when switching). */
-            Con *old_focused = TAILQ_FIRST(&(con->focus_head));
-            if (old_focused == TAILQ_END(&(con->focus_head)))
-                old_focused = NULL;
-            if (old_focused != NULL &&
-                !workspace_is_visible(con_get_workspace(old_focused)))
-                old_focused = NULL;
-
             /* 3: move the existing cons of this workspace below the new con */
+            Con **focus_order = get_focus_order(con);
+
             DLOG("Moving cons\n");
             Con *child;
             while (!TAILQ_EMPTY(&(con->nodes_head))) {
@@ -1800,13 +1848,13 @@ void con_set_layout(Con *con, layout_t layout) {
                 con_attach(child, new, true);
             }
 
+            set_focus_order(new, focus_order);
+            free(focus_order);
+
             /* 4: attach the new split container to the workspace */
             DLOG("Attaching new split to ws\n");
             con_attach(new, con, false);
 
-            if (old_focused)
-                con_activate(old_focused);
-
             tree_flatten(croot);
         }
         con_force_split_parents_redraw(con);

src/workspace.c

@@ -815,9 +815,9 @@ void ws_force_orientation(Con *ws, orientation_t orientation) {
     /* 2: copy layout from workspace */
     split->layout = ws->layout;
 
-    Con *old_focused = TAILQ_FIRST(&(ws->focus_head));
-
     /* 3: move the existing cons of this workspace below the new con */
+    Con **focus_order = get_focus_order(ws);
+
     DLOG("Moving cons\n");
     while (!TAILQ_EMPTY(&(ws->nodes_head))) {
         Con *child = TAILQ_FIRST(&(ws->nodes_head));
@@ -825,6 +825,9 @@ void ws_force_orientation(Con *ws, orientation_t orientation) {
         con_attach(child, split, true);
     }
 
+    set_focus_order(split, focus_order);
+    free(focus_order);
+
     /* 4: switch workspace layout */
     ws->layout = (orientation == HORIZ) ? L_SPLITH : L_SPLITV;
     DLOG("split->layout = %d, ws->layout = %d\n", split->layout, ws->layout);
@@ -835,9 +838,6 @@ void ws_force_orientation(Con *ws, orientation_t orientation) {
 
     /* 6: fix the percentages */
     con_fix_percent(ws);
-
-    if (old_focused)
-        con_activate(old_focused);
 }
 
 /*
@@ -892,9 +892,10 @@ Con *workspace_encapsulate(Con *ws) {
     new->parent = ws;
     new->layout = ws->layout;
 
+    Con **focus_order = get_focus_order(ws);
+
     DLOG("Moving children of workspace %p / %s into container %p\n",
          ws, ws->name, new);
-
     Con *child;
     while (!TAILQ_EMPTY(&(ws->nodes_head))) {
         child = TAILQ_FIRST(&(ws->nodes_head));
@@ -902,6 +903,9 @@ Con *workspace_encapsulate(Con *ws) {
         con_attach(child, new, true);
     }
 
+    set_focus_order(new, focus_order);
+    free(focus_order);
+
     con_attach(new, ws, true);
 
     return new;

testcases/t/294-focus-order.t

@@ -0,0 +1,109 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+#
+# Please read the following documents before working on tests:
+# • https://build.i3wm.org/docs/testsuite.html
+#   (or docs/testsuite)
+#
+# • https://build.i3wm.org/docs/lib-i3test.html
+#   (alternatively: perldoc ./testcases/lib/i3test.pm)
+#
+# • https://build.i3wm.org/docs/ipc.html
+#   (or docs/ipc)
+#
+# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf
+#   (unless you are already familiar with Perl)
+#
+# Verify that the corrent focus stack order is preserved after various
+# operations.
+use i3test;
+
+sub kill_and_confirm_focus {
+    my $focus = shift;
+    my $msg = shift;
+    cmd "kill";
+    sync_with_i3;
+    is($x->input_focus, $focus, $msg);
+}
+
+my @windows;
+
+sub focus_windows {
+    for (my $i = $#windows; $i >= 0; $i--) {
+        cmd '[id=' . $windows[$i]->id . '] focus';
+    }
+}
+
+sub confirm_focus {
+    my $msg = shift;
+    sync_with_i3;
+    is($x->input_focus, $windows[0]->id, $msg . ': window 0 focused');
+    foreach my $i (1 .. $#windows) {
+        kill_and_confirm_focus($windows[$i]->id, "$msg: window $i focused");
+    }
+    cmd 'kill';
+    @windows = ();
+}
+
+#####################################################################
+# Open 5 windows, focus them in a custom order and then change to
+# tabbed layout. The focus order should be preserved.
+#####################################################################
+
+fresh_workspace;
+
+$windows[3] = open_window;
+$windows[1] = open_window;
+$windows[0] = open_window;
+$windows[2] = open_window;
+$windows[4] = open_window;
+focus_windows;
+
+cmd 'layout tabbed';
+confirm_focus('tabbed');
+
+#####################################################################
+# Same as above but with stacked.
+#####################################################################
+
+fresh_workspace;
+
+$windows[3] = open_window;
+$windows[1] = open_window;
+$windows[0] = open_window;
+$windows[2] = open_window;
+$windows[4] = open_window;
+focus_windows;
+
+cmd 'layout stacked';
+confirm_focus('stacked');
+
+#####################################################################
+# Open 4 windows horizontally, move the last one down. The focus
+# order should be preserved.
+#####################################################################
+
+fresh_workspace;
+$windows[3] = open_window;
+$windows[2] = open_window;
+$windows[1] = open_window;
+$windows[0] = open_window;
+
+cmd 'move down';
+confirm_focus('split-h + move');
+
+#####################################################################
+# Same as above but with a vertical split.
+#####################################################################
+
+fresh_workspace;
+$windows[3] = open_window;
+cmd 'split v';
+$windows[2] = open_window;
+$windows[1] = open_window;
+$windows[0] = open_window;
+
+cmd 'move left';
+confirm_focus('split-v + move');
+
+done_testing;

testcases/t/510-focus-across-outputs.t

@@ -119,6 +119,7 @@ is($x->input_focus, $eighth->id, 'eighth window focused');
 cmd 'focus parent';
 cmd 'focus parent';
 cmd 'split v';
+cmd '[id=' . $sixth->id . '] focus';
 reset_focus $s3_ws;
 
 cmd "workspace $s1_ws";