diff --git a/include/move.h b/include/move.h index 64b12b80..df644a6b 100644 --- a/include/move.h +++ b/include/move.h @@ -17,3 +17,13 @@ * */ 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); diff --git a/src/con.c b/src/con.c index d8c30dcf..f57f0cb3 100644 --- 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/src/floating.c b/src/floating.c index 1bc4996a..9c15bb10 100644 --- a/src/floating.c +++ b/src/floating.c @@ -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; + Con *tiling_focused = con_descend_tiling_focused(ws); - /* 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_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); } diff --git a/src/move.c b/src/move.c index e8620c24..5bff3dae 100644 --- a/src/move.c +++ b/src/move.c @@ -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 diff --git a/testcases/t/135-floating-focus.t b/testcases/t/135-floating-focus.t index 97c4e4cd..168151f4 100644 --- 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 @@ -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; diff --git a/testcases/t/294-focus-order.t b/testcases/t/294-focus-order.t index 217cc844..c818f1d4 100644 --- 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;