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
This commit is contained in:
parent
e8057b2fbc
commit
252db3b8cf
|
@ -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. */
|
/* By re-attaching, the sort order will be correct afterwards. */
|
||||||
Con *previously_focused = focused;
|
Con *previously_focused = focused;
|
||||||
|
Con *previously_focused_content = focused->type == CT_WORKSPACE ? focused->parent : NULL;
|
||||||
Con *parent = workspace->parent;
|
Con *parent = workspace->parent;
|
||||||
con_detach(workspace);
|
con_detach(workspace);
|
||||||
con_attach(workspace, parent, false);
|
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);
|
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));
|
workspace_show(con_get_workspace(previously_focused));
|
||||||
|
con_activate(previously_focused);
|
||||||
|
}
|
||||||
|
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Restore the previous focus since con_attach messes with the focus. */
|
|
||||||
con_activate(previously_focused);
|
|
||||||
|
|
||||||
cmd_output->needs_tree_render = true;
|
cmd_output->needs_tree_render = true;
|
||||||
ysuccess(true);
|
ysuccess(true);
|
||||||
|
|
||||||
|
|
|
@ -94,4 +94,16 @@ cmd 'rename workspace to 5';
|
||||||
is(get_output_for_workspace('5'), 'fake-0',
|
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 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;
|
done_testing;
|
||||||
|
|
Loading…
Reference in New Issue