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 98e10c1d0815704468c5439ad35cdff27b79b966..0c5b8bcb2a2c47e121bdcfbf0a76d535efaa05b6 100644 (file)
--- 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 8a6edc857afd02e760c2197dbc38faa801531385..5c9f2ff3a70f3b8556cb8d2f7b8ca7984714d944 100644 (file)
--- 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;