]> git.sur5r.net Git - i3/i3/commitdiff
Merge pull request #2941 from orestisf1993/issue-2938
authorIngo Bürk <admin@airblader.de>
Sat, 31 Mar 2018 18:34:17 +0000 (20:34 +0200)
committerGitHub <noreply@github.com>
Sat, 31 Mar 2018 18:34:17 +0000 (20:34 +0200)
Fix focus order in floating_disable & floating_enable for unfocused windows

include/move.h
src/con.c
src/floating.c
src/move.c
testcases/t/135-floating-focus.t
testcases/t/294-focus-order.t

index 64b12b80cc7b937c8c2773989fa312a41b43adda..df644a6b8b7b217b1c3dfd8b433a00e5d94621eb 100644 (file)
  *
  */
 void tree_move(Con *con, int direction);
+
+typedef enum { BEFORE,
+               AFTER } position_t;
+
+/**
+ * This function detaches 'con' from its parent and inserts it either before or
+ * after 'target'.
+ *
+ */
+void insert_con_into(Con *con, Con *target, position_t position);
index d8c30dcf98492b8ed1707a2e9891b4b5a5ebd3a2..f57f0cb39bd094ef67da6904e0bbd60e5e6a24c1 100644 (file)
--- a/src/con.c
+++ b/src/con.c
@@ -1436,40 +1436,6 @@ orientation_t con_orientation(Con *con) {
  *
  */
 Con *con_next_focused(Con *con) {
-    Con *next;
-    /* floating containers are attached to a workspace, so we focus either the
-     * next floating container (if any) or the workspace itself. */
-    if (con->type == CT_FLOATING_CON) {
-        DLOG("selecting next for CT_FLOATING_CON\n");
-        next = TAILQ_NEXT(con, floating_windows);
-        DLOG("next = %p\n", next);
-        if (!next) {
-            next = TAILQ_PREV(con, floating_head, floating_windows);
-            DLOG("using prev, next = %p\n", next);
-        }
-        if (!next) {
-            Con *ws = con_get_workspace(con);
-            next = ws;
-            DLOG("no more floating containers for next = %p, restoring workspace focus\n", next);
-            while (next != TAILQ_END(&(ws->focus_head)) && !TAILQ_EMPTY(&(next->focus_head))) {
-                next = TAILQ_FIRST(&(next->focus_head));
-                if (next == con) {
-                    DLOG("skipping container itself, we want the next client\n");
-                    next = TAILQ_NEXT(next, focused);
-                }
-            }
-            if (next == TAILQ_END(&(ws->focus_head))) {
-                DLOG("Focus list empty, returning ws\n");
-                next = ws;
-            }
-        } else {
-            /* Instead of returning the next CT_FLOATING_CON, we descend it to
-             * get an actual window to focus. */
-            next = con_descend_focused(next);
-        }
-        return next;
-    }
-
     /* dock clients cannot be focused, so we focus the workspace instead */
     if (con->parent->type == CT_DOCKAREA) {
         DLOG("selecting workspace for dock client\n");
@@ -1478,10 +1444,9 @@ Con *con_next_focused(Con *con) {
 
     /* if 'con' is not the first entry in the focus stack, use the first one as
      * it’s currently focused already */
-    Con *first = TAILQ_FIRST(&(con->parent->focus_head));
-    if (first != con) {
-        DLOG("Using first entry %p\n", first);
-        next = first;
+    Con *next = TAILQ_FIRST(&(con->parent->focus_head));
+    if (next != con) {
+        DLOG("Using first entry %p\n", next);
     } else {
         /* try to focus the next container on the same level as this one or fall
          * back to its parent */
@@ -1496,6 +1461,10 @@ Con *con_next_focused(Con *con) {
         next = TAILQ_FIRST(&(next->focus_head));
     }
 
+    if (con->type == CT_FLOATING_CON && next != con->parent) {
+        next = con_descend_focused(next);
+    }
+
     return next;
 }
 
index 1bc4996a17c407868ff4e12d6e3be16463fd5b45..9c15bb10e243ca1eec3c1289648107f0c57948ab 100644 (file)
@@ -176,11 +176,36 @@ void floating_enable(Con *con, bool automatic) {
         return;
     }
 
+    Con *focus_head_placeholder = NULL;
+    bool focus_before_parent = true;
+    if (!set_focus) {
+        /* Find recursively the ancestor container which is a child of our workspace.
+         * We need to reuse its focus position later. */
+        Con *ancestor = con;
+        while (ancestor->parent->type != CT_WORKSPACE) {
+            focus_before_parent &= TAILQ_FIRST(&(ancestor->parent->focus_head)) == ancestor;
+            ancestor = ancestor->parent;
+        }
+        /* Consider the part of the focus stack of our current workspace:
+         * [ ... S_{i-1} S_{i} S_{i+1} ... ]
+         * Where S_{x} is a container tree and the container 'con' that is beeing switched to
+         * floating belongs in S_{i}. The new floating container, 'nc', will have the
+         * workspace as its parent so it needs to be placed in this stack. If C was focused
+         * we just need to call con_focus(). Otherwise, nc must be placed before or after S_{i}.
+         * We should avoid using the S_{i} container for our operations since it might get
+         * killed if it has no other children. So, the two possible positions are after S_{i-1}
+         * or before S_{i+1}.
+         */
+        if (focus_before_parent) {
+            focus_head_placeholder = TAILQ_PREV(ancestor, focus_head, focused);
+        } else {
+            focus_head_placeholder = TAILQ_NEXT(ancestor, focused);
+        }
+    }
+
     /* 1: detach the container from its parent */
     /* TODO: refactor this with tree_close_internal() */
-    TAILQ_REMOVE(&(con->parent->nodes_head), con, nodes);
-    TAILQ_REMOVE(&(con->parent->focus_head), con, focused);
-
+    con_detach(con);
     con_fix_percent(con->parent);
 
     /* 2: create a new container to render the decoration on, add
@@ -196,12 +221,23 @@ void floating_enable(Con *con, bool automatic) {
     /* We insert nc already, even though its rect is not yet calculated. This
      * is necessary because otherwise the workspace might be empty (and get
      * closed in tree_close_internal()) even though it’s not. */
-    if (set_focus) {
-        TAILQ_INSERT_TAIL(&(ws->floating_head), nc, floating_windows);
+    TAILQ_INSERT_HEAD(&(ws->floating_head), nc, floating_windows);
+
+    struct focus_head *fh = &(ws->focus_head);
+    if (focus_before_parent) {
+        if (focus_head_placeholder) {
+            TAILQ_INSERT_AFTER(fh, focus_head_placeholder, nc, focused);
+        } else {
+            TAILQ_INSERT_HEAD(fh, nc, focused);
+        }
     } else {
-        TAILQ_INSERT_HEAD(&(ws->floating_head), nc, floating_windows);
+        if (focus_head_placeholder) {
+            TAILQ_INSERT_BEFORE(focus_head_placeholder, nc, focused);
+        } else {
+            /* Also used for the set_focus case */
+            TAILQ_INSERT_TAIL(fh, nc, focused);
+        }
     }
-    TAILQ_INSERT_TAIL(&(ws->focus_head), nc, focused);
 
     /* check if the parent container is empty and close it if so */
     if ((con->parent->type == CT_CON || con->parent->type == CT_FLOATING_CON) &&
@@ -329,45 +365,22 @@ void floating_disable(Con *con, bool automatic) {
         return;
     }
 
-    const bool set_focus = (con == focused);
-
     Con *ws = con_get_workspace(con);
-    Con *parent = con->parent;
-
-    /* 1: detach from parent container */
-    TAILQ_REMOVE(&(con->parent->nodes_head), con, nodes);
-    TAILQ_REMOVE(&(con->parent->focus_head), con, focused);
-
-    /* 2: kill parent container */
-    TAILQ_REMOVE(&(con->parent->parent->floating_head), con->parent, floating_windows);
-    TAILQ_REMOVE(&(con->parent->parent->focus_head), con->parent, focused);
-    /* clear the pointer before calling tree_close_internal in which the memory is freed */
-    con->parent = NULL;
-    tree_close_internal(parent, DONT_KILL_WINDOW, true, false);
-
-    /* 3: re-attach to the parent of the currently focused con on the workspace
-     * this floating con was on */
-    Con *focused = con_descend_tiling_focused(ws);
-
-    /* if there is no other container on this workspace, focused will be the
-     * workspace itself */
-    if (focused->type == CT_WORKSPACE)
-        con->parent = focused;
-    else
-        con->parent = focused->parent;
+    Con *tiling_focused = con_descend_tiling_focused(ws);
 
-    /* con_fix_percent will adjust the percent value */
-    con->percent = 0.0;
+    if (tiling_focused->type == CT_WORKSPACE) {
+        Con *parent = con->parent;
+        con_detach(con);
+        con->parent = NULL;
+        tree_close_internal(parent, DONT_KILL_WINDOW, true, false);
+        con_attach(con, tiling_focused, false);
+        con->percent = 0.0;
+        con_fix_percent(con->parent);
+    } else {
+        insert_con_into(con, tiling_focused, AFTER);
+    }
 
     con->floating = FLOATING_USER_OFF;
-
-    con_attach(con, con->parent, false);
-
-    con_fix_percent(con->parent);
-
-    if (set_focus)
-        con_activate(con);
-
     floating_set_hint_atom(con, false);
     ipc_send_window_event("floating", con);
 }
index e8620c247b0339d4b0fe529ef97108816ad703c9..5bff3dae5030bc844d915ac8a14d28c71b34876b 100644 (file)
@@ -9,9 +9,6 @@
  */
 #include "all.h"
 
-typedef enum { BEFORE,
-               AFTER } position_t;
-
 /*
  * Returns the lowest container in the tree that has both a and b as descendants.
  *
@@ -65,7 +62,7 @@ static bool is_focused_descendant(Con *con, Con *ancestor) {
  * after 'target'.
  *
  */
-static void insert_con_into(Con *con, Con *target, position_t position) {
+void insert_con_into(Con *con, Con *target, position_t position) {
     Con *parent = target->parent;
     /* We need to preserve the old con->parent. While it might still be used to
      * insert the entry before/after it, we call the on_remove_child callback
index 97c4e4cd3d2429fd0ad1a756c4b30866fe94ce27..168151f4c0191018883f4e741248116614f6efb6 100644 (file)
@@ -105,18 +105,14 @@ cmd 'split v';
 cmd 'layout stacked';
 $second = open_window({ background_color => '#00ff00' });   # window 6
 $third = open_window({ background_color => '#0000ff' }); # window 7
-
 is($x->input_focus, $third->id, 'last container focused');
 
-cmd 'floating enable';
-
 cmd '[id="' . $second->id . '"] focus';
-
-is($x->input_focus, $second->id, 'second con focused');
-
 cmd 'floating enable';
+cmd '[id="' . $third->id . '"] floating enable';
 
 sync_with_i3;
+is($x->input_focus, $second->id, 'second con focused');
 
 # now kill the second one. focus should fall back to the third one, which is
 # also floating
@@ -240,4 +236,184 @@ $ws = get_ws($tmp);
 is($ws->{floating_nodes}->[1]->{nodes}->[0]->{window}, $first->id, 'first on top');
 is($ws->{floating_nodes}->[0]->{nodes}->[0]->{window}, $second->id, 'second behind');
 
+#############################################################################
+# 9: verify that disabling / enabling floating for a window from a different
+# workspace maintains the correct focus order.
+#############################################################################
+
+sub open_window_helper {
+    my $floating = shift if @_;
+    if ($floating){
+        return open_floating_window;
+    }
+    else {
+        return open_window;
+    }
+}
+
+for my $floating (0, 1){
+    $tmp = fresh_workspace;
+    $first = open_window;
+    $second = open_window_helper($floating);
+    is($x->input_focus, $second->id, "second window focused");
+
+    fresh_workspace;
+    cmd "[id=" . $second->id . "] floating toggle";
+    cmd "workspace $tmp";
+    sync_with_i3;
+
+    my $workspace = get_ws($tmp);
+    is($workspace->{floating_nodes}->[0]->{nodes}->[0]->{window}, $second->id, 'second window on first workspace, floating') unless $floating;
+    is($workspace->{nodes}->[1]->{window}, $second->id, 'second window on first workspace, right') unless !$floating;
+    is($x->input_focus, $second->id, 'second window still focused');
+}
+
+#############################################################################
+# 10: verify that toggling floating for an unfocused window on another
+# workspace doesn't make it focused.
+#############################################################################
+
+for my $floating (0, 1){
+    $tmp = fresh_workspace;
+    $first = open_window_helper($floating);
+    $second = open_window;
+    is($x->input_focus, $second->id, 'second (tiling) window focused');
+
+    fresh_workspace;
+    cmd "[id=" . $first->id . "] floating toggle";
+    cmd "workspace $tmp";
+    sync_with_i3;
+
+    my $workspace = get_ws($tmp);
+    is($workspace->{floating_nodes}->[0]->{nodes}->[0]->{window}, $first->id, 'first window on first workspace, floating') unless $floating;
+    is($workspace->{nodes}->[1]->{window}, $first->id, 'first window on first workspace, right') unless !$floating;
+    is($x->input_focus, $second->id, 'second window still focused');
+}
+
+#############################################################################
+# 11: verify that toggling floating for a focused window on another workspace
+# which has another, unfocused floating window maintains the focus of the
+# first window.
+#############################################################################
+for my $floating (0, 1){
+    $tmp = fresh_workspace;
+    $first = open_window;
+    $second = open_floating_window;
+    is($x->input_focus, $second->id, 'second (floating) window focused');
+    $third = open_window_helper($floating);
+    is($x->input_focus, $third->id, "third (floating = $floating) window focused");
+
+    fresh_workspace;
+    cmd "[id=" . $third->id . "] floating toggle";
+    cmd "workspace $tmp";
+    sync_with_i3;
+
+    my $workspace = get_ws($tmp);
+    is($workspace->{floating_nodes}->[0]->{nodes}->[0]->{window}, $third->id, 'third window on first workspace, floating') unless $floating;
+    is($workspace->{nodes}->[1]->{window}, $third->id, 'third window on first workspace, right') unless !$floating;
+    is($x->input_focus, $third->id, 'third window still focused');
+}
+
+#############################################################################
+# 12: verify that toggling floating for an unfocused window on another
+# workspace which has another, focused floating window doesn't change focus.
+#############################################################################
+
+for my $floating (0, 1){
+    $tmp = fresh_workspace;
+    $first = open_window;
+    $second = open_window_helper($floating);
+    is($x->input_focus, $second->id, "second (floating = $floating) window focused");
+    $third = open_floating_window;
+    is($x->input_focus, $third->id, 'third (floating) window focused');
+
+    fresh_workspace;
+    cmd "[id=" . $second->id . "] floating toggle";
+    cmd "workspace $tmp";
+    sync_with_i3;
+
+    my $workspace = get_ws($tmp);
+    is($workspace->{floating_nodes}->[0]->{nodes}->[0]->{window}, $second->id, 'second window on first workspace, floating') unless $floating;
+    is($workspace->{nodes}->[1]->{window}, $second->id, 'second window on first workspace, right') unless !$floating;
+    is($x->input_focus, $third->id, 'third window still focused');
+}
+
+#############################################################################
+# 13: For layout [H1 [A V1[ B F ] ] ] verify that toggling F's floating
+# mode maintains its focus.
+#############################################################################
+
+for my $floating (0, 1){
+    $tmp = fresh_workspace;
+    $first = open_window;
+    $second = open_window;
+    cmd "split v";
+    sync_with_i3;
+    is($x->input_focus, $second->id, "second (floating = $floating) window focused");
+    $third = open_window_helper($floating);
+    is($x->input_focus, $third->id, 'third (floating) window focused');
+
+    fresh_workspace;
+    cmd "[id=" . $third->id . "] floating toggle";
+    cmd "workspace $tmp";
+    sync_with_i3;
+
+    my $workspace = get_ws($tmp);
+    is($workspace->{floating_nodes}->[0]->{nodes}->[0]->{window}, $third->id, 'third window on first workspace, floating') unless $floating;
+    is($workspace->{nodes}->[1]->{nodes}->[1]->{window}, $third->id, 'third window on first workspace') unless !$floating;
+    is($x->input_focus, $third->id, 'third window still focused');
+}
+
+#############################################################################
+# 14: For layout [H1 [A V1[ H2 [B H2 [ C V2 [ F D ] ] ] ] ] ] verify that
+# toggling F's floating mode maintains its focus.
+#############################################################################
+
+sub kill_and_confirm_focus {
+    my $focus = shift;
+    my $msg = shift;
+    cmd "kill";
+    sync_with_i3;
+    is($x->input_focus, $focus, $msg);
+}
+
+$tmp = fresh_workspace;
+my $A = open_window;
+my $B = open_window;
+cmd "split v";
+my $C = open_window;
+cmd "split h";
+my $F = open_window;
+cmd "split v";
+my $D = open_window;
+is($x->input_focus, $D->id, "D is focused");
+
+sync_with_i3;
+my $workspace = get_ws($tmp);
+is($workspace->{nodes}->[1]->{nodes}->[1]->{nodes}->[1]->{nodes}->[0]->{window}, $F->id, 'F opened in its expected position');
+
+fresh_workspace;
+cmd "[id=" . $F->id . "] floating enable";
+cmd "workspace $tmp";
+sync_with_i3;
+
+$workspace = get_ws($tmp);
+is($workspace->{floating_nodes}->[0]->{nodes}->[0]->{window}, $F->id, 'F on first workspace, floating');
+is($workspace->{nodes}->[1]->{nodes}->[1]->{nodes}->[1]->{nodes}->[0]->{window}, $D->id, 'D where F used to be');
+is($x->input_focus, $D->id, 'D still focused');
+
+fresh_workspace;
+cmd "[id=" . $F->id . "] floating disable";
+cmd "workspace $tmp";
+sync_with_i3;
+
+$workspace = get_ws($tmp);
+is($workspace->{nodes}->[1]->{nodes}->[1]->{nodes}->[1]->{nodes}->[1]->{window}, $F->id, 'F where D used to be');
+is($x->input_focus, $D->id, 'D still focused');
+
+kill_and_confirm_focus($F->id, 'F focused after D is killed');
+kill_and_confirm_focus($C->id, 'C focused after F is killed');
+kill_and_confirm_focus($B->id, 'B focused after C is killed');
+kill_and_confirm_focus($A->id, 'A focused after B is killed');
+
 done_testing;
index 217cc8441ba412f012e0828056686292c2d15072..c818f1d441e36aea832e802163c2f42de8604f6b 100644 (file)
@@ -176,4 +176,35 @@ cmd '[id=' . $windows[2]->id . '] move to workspace ' . $ws;
 cmd '[id=' . $windows[1]->id . '] move to workspace ' . $ws;
 confirm_focus('\'move to workspace\' focus order when moving containers from other workspace');
 
+######################################################################
+# Test focus order with floating and tiling windows.
+# See issue: 1975
+######################################################################
+
+fresh_workspace;
+$windows[2] = open_window;
+$windows[0] = open_window;
+$windows[3] = open_floating_window;
+$windows[1] = open_floating_window;
+focus_windows;
+
+confirm_focus('mix of floating and tiling windows');
+
+######################################################################
+# Same but an unfocused tiling window is killed first.
+######################################################################
+
+fresh_workspace;
+$windows[2] = open_window;
+$windows[0] = open_window;
+$windows[3] = open_floating_window;
+$windows[1] = open_floating_window;
+focus_windows;
+
+cmd '[id=' . $windows[1]->id . '] focus';
+cmd '[id=' . $windows[0]->id . '] kill';
+
+kill_and_confirm_focus($windows[2]->id, 'window 2 focused after tiling killed');
+kill_and_confirm_focus($windows[3]->id, 'window 3 focused after tiling killed');
+
 done_testing;