From ba8c64c4d9f6d8a8efa8763c6d71f7e6460b712f Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Thu, 11 Oct 2018 01:21:56 +0300 Subject: [PATCH 1/3] 285-sticky.t: Use kill_all_windows --- testcases/t/285-sticky.t | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/testcases/t/285-sticky.t b/testcases/t/285-sticky.t index f53e4a93..927d3d74 100644 --- a/testcases/t/285-sticky.t +++ b/testcases/t/285-sticky.t @@ -25,41 +25,41 @@ my ($ws, $tmp, $focused); # nothing happens. ############################################################################### fresh_workspace; -open_window(wm_class => 'findme'); +open_window; cmd 'sticky enable'; $ws = fresh_workspace; is(@{get_ws($ws)->{nodes}}, 0, 'tiling sticky container did not move'); is(@{get_ws($ws)->{floating_nodes}}, 0, 'tiling sticky container did not move'); -cmd '[class="findme"] kill'; +kill_all_windows; ############################################################################### # 2: Given a sticky floating container, when the workspace is switched, then # the container moves to the new workspace. ############################################################################### $ws = fresh_workspace; -open_floating_window(wm_class => 'findme'); +open_floating_window; $focused = get_focused($ws); cmd 'sticky enable'; $ws = fresh_workspace; is(@{get_ws($ws)->{floating_nodes}}, 1, 'floating sticky container moved to new workspace'); is(get_focused($ws), $focused, 'sticky container has focus'); -cmd '[class="findme"] kill'; +kill_all_windows; ############################################################################### # 3: Given two sticky floating containers, when the workspace is switched, # then both containers move to the new workspace. ############################################################################### fresh_workspace; -open_floating_window(wm_class => 'findme'); +open_floating_window; cmd 'sticky enable'; -open_floating_window(wm_class => 'findme'); +open_floating_window; cmd 'sticky enable'; $ws = fresh_workspace; is(@{get_ws($ws)->{floating_nodes}}, 2, 'multiple sticky windows can be used at the same time'); -cmd '[class="findme"] kill'; +kill_all_windows; ############################################################################### # 4: Given an unfocused sticky floating container and a tiling container on the @@ -70,13 +70,13 @@ $ws = fresh_workspace; open_window; $focused = get_focused($ws); fresh_workspace; -open_floating_window(wm_class => 'findme'); +open_floating_window; cmd 'sticky enable'; open_window; cmd 'workspace ' . $ws; is(get_focused($ws), $focused, 'the tiling container has focus'); -cmd '[class="findme"] kill'; +kill_all_windows; ############################################################################### # 5: Given a focused sticky floating container and a tiling container on the @@ -86,13 +86,13 @@ cmd '[class="findme"] kill'; $ws = fresh_workspace; open_window; $tmp = fresh_workspace; -open_floating_window(wm_class => 'findme'); +open_floating_window; $focused = get_focused($tmp); cmd 'sticky enable'; cmd 'workspace ' . $ws; is(get_focused($ws), $focused, 'the sticky container has focus'); -cmd '[class="findme"] kill'; +kill_all_windows; ############################################################################### # 6: Given a floating container on a non-visible workspace, when the window @@ -100,13 +100,13 @@ cmd '[class="findme"] kill'; # visible workspace. ############################################################################### fresh_workspace; -open_floating_window(wm_class => 'findme'); +open_floating_window; cmd 'mark sticky'; $ws = fresh_workspace; cmd '[con_mark=sticky] sticky enable'; is(@{get_ws($ws)->{floating_nodes}}, 1, 'the sticky window jumps to the front'); -cmd '[class="findme"] kill'; +kill_all_windows; ############################################################################### From 6728696ec8f92f8e9c7591fe4d242dd90da977cb Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Thu, 11 Oct 2018 12:05:31 +0300 Subject: [PATCH 2/3] output_push_sticky_windows: Make a bit easier to understand --- include/output.h | 9 ++++++++- src/output.c | 28 +++++++++++++++++----------- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/include/output.h b/include/output.h index 31084da1..a2ad97b0 100644 --- a/include/output.h +++ b/include/output.h @@ -40,5 +40,12 @@ Output *get_output_for_con(Con *con); * Iterates over all outputs and pushes sticky windows to the currently visible * workspace on that output. * + * old_focus is used to determine if a sticky window is going to be focused. + * old_focus might be different than the currently focused container because the + * caller might need to temporarily change the focus and then call + * output_push_sticky_windows. For example, workspace_show needs to set focus to + * one of its descendants first, then call output_push_sticky_windows that + * should focus a sticky window if it was the focused in the previous workspace. + * */ -void output_push_sticky_windows(Con *to_focus); +void output_push_sticky_windows(Con *old_focus); diff --git a/src/output.c b/src/output.c index 19a7c4af..ebba5c77 100644 --- a/src/output.c +++ b/src/output.c @@ -72,8 +72,15 @@ Output *get_output_for_con(Con *con) { * Iterates over all outputs and pushes sticky windows to the currently visible * workspace on that output. * + * old_focus is used to determine if a sticky window is going to be focused. + * old_focus might be different than the currently focused container because the + * caller might need to temporarily change the focus and then call + * output_push_sticky_windows. For example, workspace_show needs to set focus to + * one of its descendants first, then call output_push_sticky_windows that + * should focus a sticky window if it was the focused in the previous workspace. + * */ -void output_push_sticky_windows(Con *to_focus) { +void output_push_sticky_windows(Con *old_focus) { Con *output; TAILQ_FOREACH(output, &(croot->focus_head), focused) { Con *workspace, *visible_ws = NULL; @@ -95,18 +102,17 @@ void output_push_sticky_windows(Con *to_focus) { child != TAILQ_END(&(current_ws->focus_head));) { Con *current = child; child = TAILQ_NEXT(child, focused); - if (current->type != CT_FLOATING_CON) + if (current->type != CT_FLOATING_CON || !con_is_sticky(current)) { continue; + } - if (con_is_sticky(current)) { - bool ignore_focus = (to_focus == NULL) || (current != to_focus->parent); - con_move_to_workspace(current, visible_ws, true, false, ignore_focus); - if (!ignore_focus) { - Con *current_ws = con_get_workspace(focused); - con_activate(con_descend_focused(current)); - /* Pushing sticky windows shouldn't change the focused workspace. */ - con_activate(con_descend_focused(current_ws)); - } + bool ignore_focus = (old_focus == NULL) || (current != old_focus->parent); + con_move_to_workspace(current, visible_ws, true, false, ignore_focus); + if (!ignore_focus) { + Con *current_ws = con_get_workspace(focused); + con_activate(con_descend_focused(current)); + /* Pushing sticky windows shouldn't change the focused workspace. */ + con_activate(con_descend_focused(current_ws)); } } } From b09090fa7d2c8c278181c1b2c9429dcfec6a8b35 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Thu, 11 Oct 2018 12:06:17 +0300 Subject: [PATCH 3/3] Fix sticky focus when switching to workspace on different output See the testcase for the exact steps to reproduce the problem. --- src/workspace.c | 8 +++++++- testcases/t/285-sticky.t | 27 ++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/workspace.c b/src/workspace.c index 5f5c8d4f..6fe9a128 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -410,7 +410,6 @@ static void workspace_defer_update_urgent_hint_cb(EV_P_ ev_timer *w, int revents */ void workspace_show(Con *workspace) { Con *current, *old = NULL; - Con *old_focus = focused; /* safe-guard against showing i3-internal workspaces like __i3_scratch */ if (con_is_internal(workspace)) @@ -433,6 +432,13 @@ void workspace_show(Con *workspace) { return; } + /* Used to correctly update focus when pushing sticky windows. Holds the + * previously focused container in the same output as workspace. For + * example, if a sticky window is focused and then we switch focus to a + * workspace in another output and then switch to a third workspace in the + * first output, the sticky window needs to be refocused. */ + Con *old_focus = old ? con_descend_focused(old) : NULL; + /* Remember currently focused workspace for switching back to it later with * the 'workspace back_and_forth' command. * NOTE: We have to duplicate the name as the original will be freed when diff --git a/testcases/t/285-sticky.t b/testcases/t/285-sticky.t index 927d3d74..8dfe9aea 100644 --- a/testcases/t/285-sticky.t +++ b/testcases/t/285-sticky.t @@ -16,7 +16,14 @@ # # Tests sticky windows. # Ticket: #1455 -use i3test; +use i3test i3_config => <{floating_nodes}}, 1, 'the sticky window jumps to the front'); kill_all_windows; +############################################################################### +# 7: Given a sticky floating container and a workspace on another output, when +# a new workspace assigned to the first output is focused, then the sticky +# container should jump to the new workspace and have input focus correctly. +############################################################################### +$ws = fresh_workspace(output => 0); +open_floating_window; +cmd 'sticky enabled'; +$focused = get_focused($ws); +$ws = fresh_workspace(output => 1); + +is(@{get_ws($ws)->{floating_nodes}}, 0, 'the sticky window didn\'t jump to a workspace on a different output'); +$ws = 'ws-on-0'; +cmd "workspace $ws"; +is(@{get_ws($ws)->{floating_nodes}}, 1, 'the sticky window moved to new workspace on first output'); +is(get_focused($ws), $focused, 'the sticky window has focus'); +kill_all_windows; + ############################################################################### done_testing;