From cb73fd5e31eefb3243d70db4026253e7bcafb060 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Wed, 13 Dec 2017 16:15:01 +0200 Subject: [PATCH] Fix focus order issues when encapsulating workspaces See new tests for an explanation of the problem. --- src/con.c | 18 ++-- src/workspace.c | 16 ++-- testcases/t/294-focus-order.t | 109 +++++++++++++++++++++++++ testcases/t/510-focus-across-outputs.t | 1 + 4 files changed, 125 insertions(+), 19 deletions(-) create mode 100644 testcases/t/294-focus-order.t diff --git a/src/con.c b/src/con.c index 47e9a342..985d07da 100644 --- a/src/con.c +++ b/src/con.c @@ -1837,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))) { @@ -1856,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); diff --git a/src/workspace.c b/src/workspace.c index d200b6e4..cbef473b 100644 --- a/src/workspace.c +++ b/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; diff --git a/testcases/t/294-focus-order.t b/testcases/t/294-focus-order.t new file mode 100644 index 00000000..41c0bf07 --- /dev/null +++ b/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; diff --git a/testcases/t/510-focus-across-outputs.t b/testcases/t/510-focus-across-outputs.t index 07a2115c..065437f0 100644 --- a/testcases/t/510-focus-across-outputs.t +++ b/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";