Remove special handling of floating containers in con_next_focused

Explanation for the changed test:
After $third is switched to floating, the test moves focus to $second.
So, the parent of $second (the stacked container) is above $third in the
focus stack and it's children ($first, $second) should get focused
before $second. When $second is switched to floating the correct focus
order for the workspace should be $second->parent (floating con is the
parent) > $first->parent (stacked con) > $third.

Fixes #1975

diff --git a/src/con.c b/src/con.c
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;
 }
 

diff --git a/testcases/t/135-floating-focus.t b/testcases/t/135-floating-focus.t
index 282bab43d26f9ab62212a435ae245bd5d78f3b00..168151f4c0191018883f4e741248116614f6efb6 100644 (file)
--- a/testcases/t/135-floating-focus.t
+++ b/testcases/t/135-floating-focus.t
@@ -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

diff --git a/testcases/t/294-focus-order.t b/testcases/t/294-focus-order.t
index 217cc8441ba412f012e0828056686292c2d15072..c818f1d441e36aea832e802163c2f42de8604f6b 100644 (file)
--- a/testcases/t/294-focus-order.t
+++ b/testcases/t/294-focus-order.t
@@ -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;