Skip to content

Commit

Permalink
Don't refocus a workspace cleaned up by workspace_show during rename
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ograff authored and orestisfl committed May 1, 2018
1 parent e8057b2 commit 252db3b
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 4 deletions.
22 changes: 18 additions & 4 deletions src/commands.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);

Expand Down
12 changes: 12 additions & 0 deletions testcases/t/522-rename-assigned-workspace.t
Original file line number Diff line number Diff line change
Expand Up @@ -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;

0 comments on commit 252db3b

Please sign in to comment.