From: Oliver Graff Date: Tue, 1 May 2018 08:25:13 +0000 (-0400) Subject: Don't refocus a workspace cleaned up by `workspace_show` during rename X-Git-Tag: 4.16~85 X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=252db3b8cfad6aa3c9cf22f51eb2a585ae69c791;p=i3%2Fi3 Don't refocus a workspace cleaned up by `workspace_show` during rename When moving a workspace to the current output by way of a rename, if the current workspace is empty, it will be removed by `workspace_show`. Attempting to restore focus to this removed workspace causes a crash. Follow the pattern in workspace.c:996 to only restore the original focus if the original workspace still exists. Add a test to ensure that the renamed workspace moves to its appropriate output and that a crash does not occur. Fixes #3228 --- diff --git a/src/commands.c b/src/commands.c index 98e10c1d..0c5b8bcb 100644 --- a/src/commands.c +++ b/src/commands.c @@ -2000,6 +2000,7 @@ void cmd_rename_workspace(I3_CMD, const char *old_name, const char *new_name) { /* By re-attaching, the sort order will be correct afterwards. */ Con *previously_focused = focused; + Con *previously_focused_content = focused->type == CT_WORKSPACE ? focused->parent : NULL; Con *parent = workspace->parent; con_detach(workspace); con_attach(workspace, parent, false); @@ -2020,15 +2021,28 @@ void cmd_rename_workspace(I3_CMD, const char *old_name, const char *new_name) { } workspace_move_to_output(workspace, target_output); - if (previously_focused) + bool can_restore_focus = previously_focused != NULL; + /* NB: If previously_focused is a workspace we can't + * work directly with it since it might have been cleaned up by + * workspace_show() already, depending on the + * focus order/number of other workspaces on the output. + * Instead, we loop through the available workspaces and only focus + * previously_focused if we still find it. */ + if (previously_focused_content) { + Con *workspace = NULL; + GREP_FIRST(workspace, previously_focused_content, child == previously_focused); + can_restore_focus &= (workspace != NULL); + } + + if (can_restore_focus) { + /* Restore the previous focus since con_attach messes with the focus. */ workspace_show(con_get_workspace(previously_focused)); + con_activate(previously_focused); + } break; } - /* Restore the previous focus since con_attach messes with the focus. */ - con_activate(previously_focused); - cmd_output->needs_tree_render = true; ysuccess(true); diff --git a/testcases/t/522-rename-assigned-workspace.t b/testcases/t/522-rename-assigned-workspace.t index 8a6edc85..5c9f2ff3 100644 --- a/testcases/t/522-rename-assigned-workspace.t +++ b/testcases/t/522-rename-assigned-workspace.t @@ -94,4 +94,16 @@ cmd 'rename workspace to 5'; is(get_output_for_workspace('5'), 'fake-0', 'Renaming the workspace to a workspace assigned to a directional output should not move the workspace'); +########################################################################## +# Renaming a workspace, so that it becomes assigned to the focused +# output's workspace (and the focused output is empty) should +# result in the original workspace replacing the originally +# focused workspace. +########################################################################## + +cmd 'workspace baz'; +cmd 'rename workspace 5 to 2'; +is(get_output_for_workspace('2'), 'fake-1', + 'Renaming a workspace so that it moves to the focused output which contains only an empty workspace should replace the empty workspace'); + done_testing;