Bugfix: Correctly revert focus to other floating windows when closing a floating...

Uncovered by the testsuite \o/

diff --git a/include/tree.h b/include/tree.h
index b9bf7f54ae0ce9baeee18803a69bd859f0becb36..b483434fcdaad90891cbdf09d8fd5a4f8f34ade7 100644 (file)
--- a/include/tree.h
+++ b/include/tree.h
@@ -70,8 +70,15 @@ void tree_next(char way, orientation_t orientation);
  * Returns true if the container was killed or false if just WM_DELETE was sent
  * and the window is expected to kill itself.
  *
+ * The dont_kill_parent flag is specified when the function calls itself
+ * recursively while deleting a containers children.
+ *
+ * The force_set_focus flag is specified in the case of killing a floating
+ * window: tree_close() will be invoked for the CT_FLOATINGCON (the parent
+ * container) and focus should be set there.
+ *
  */
-bool tree_close(Con *con, kill_window_t kill_window, bool dont_kill_parent);
+bool tree_close(Con *con, kill_window_t kill_window, bool dont_kill_parent, bool force_set_focus);
 
 /**
  * Loads tree from ~/.i3/_restart.json (used for in-place restarts).

diff --git a/src/cmdparse.y b/src/cmdparse.y
index 9ea82efd68c5f8e6a77a4998831b8a8b1a116213..bf2fde961c46876bf92135da72bbf6b4cb4e8869 100644 (file)
--- a/src/cmdparse.y
+++ b/src/cmdparse.y
@@ -533,7 +533,7 @@ kill:
         else {
             TAILQ_FOREACH(current, &owindows, owindows) {
                 printf("matching: %p / %s\n", current->con, current->con->name);
-                tree_close(current->con, $2, false);
+                tree_close(current->con, $2, false, false);
             }
         }
 

diff --git a/src/con.c b/src/con.c
index 148f782277fc40f18c180ada099f9419c7c69fbc..8fbedd3d608561e9346942184335256a65d2a0c1 100644 (file)
--- a/src/con.c
+++ b/src/con.c
@@ -1021,7 +1021,7 @@ static void con_on_remove_child(Con *con) {
     int children = con_num_children(con);
     if (children == 0) {
         DLOG("Container empty, closing\n");
-        tree_close(con, DONT_KILL_WINDOW, false);
+        tree_close(con, DONT_KILL_WINDOW, false, false);
         return;
     }
 }

diff --git a/src/floating.c b/src/floating.c
index 8d3ce4e244a9059ebfe20c6f981ed31d028180f4..b898d7bc6c901823191dc6a676f5681545aa48d1 100644 (file)
--- a/src/floating.c
+++ b/src/floating.c
@@ -91,7 +91,7 @@ void floating_enable(Con *con, bool automatic) {
     /* check if the parent container is empty and close it if so */
     if ((con->parent->type == CT_CON || con->parent->type == CT_FLOATING_CON) && con_num_children(con->parent) == 0) {
         DLOG("Old container empty after setting this child to floating, closing\n");
-        tree_close(con->parent, DONT_KILL_WINDOW, false);
+        tree_close(con->parent, DONT_KILL_WINDOW, false, false);
     }
 
     char *name;
@@ -212,7 +212,7 @@ void floating_disable(Con *con, bool automatic) {
     /* 2: kill parent container */
     TAILQ_REMOVE(&(con->parent->parent->floating_head), con->parent, floating_windows);
     TAILQ_REMOVE(&(con->parent->parent->focus_head), con->parent, focused);
-    tree_close(con->parent, DONT_KILL_WINDOW, false);
+    tree_close(con->parent, DONT_KILL_WINDOW, false, false);
 
     /* 3: re-attach to the parent of the currently focused con on the workspace
      * this floating con was on */

diff --git a/src/handlers.c b/src/handlers.c
index b346799033a3ffa75418a0079301b02bcde06178..336a491f67f6b27300c6cff50226136e9ea3abc6 100644 (file)
--- a/src/handlers.c
+++ b/src/handlers.c
@@ -457,7 +457,7 @@ static int handle_unmap_notify_event(xcb_unmap_notify_event_t *event) {
         return 1;
     }
 
-    tree_close(con, DONT_KILL_WINDOW, false);
+    tree_close(con, DONT_KILL_WINDOW, false, false);
     tree_render();
     x_push_changes(croot);
     return 1;

diff --git a/src/randr.c b/src/randr.c
index 6b6cd67d0e35ba524def157e7c0fa138c1f8691d..6da90070589d18b33e2aacbd79b7b8b71a3935ce 100644 (file)
--- a/src/randr.c
+++ b/src/randr.c
@@ -789,7 +789,7 @@ void randr_query_outputs() {
                 }
 
                 DLOG("destroying disappearing con %p\n", output->con);
-                tree_close(output->con, DONT_KILL_WINDOW, true);
+                tree_close(output->con, DONT_KILL_WINDOW, true, false);
                 DLOG("Done. Should be fine now\n");
                 output->con = NULL;
             }

diff --git a/src/tree.c b/src/tree.c
index 0d0c2e9fa5ff644fc97d9239015241914a4afd6e..76530d604bb1f3546abe89695ab141cbf5abfa6e 100644 (file)
--- a/src/tree.c
+++ b/src/tree.c
@@ -114,8 +114,15 @@ static bool _is_con_mapped(Con *con) {
  * Returns true if the container was killed or false if just WM_DELETE was sent
  * and the window is expected to kill itself.
  *
+ * The dont_kill_parent flag is specified when the function calls itself
+ * recursively while deleting a containers children.
+ *
+ * The force_set_focus flag is specified in the case of killing a floating
+ * window: tree_close() will be invoked for the CT_FLOATINGCON (the parent
+ * container) and focus should be set there.
+ *
  */
-bool tree_close(Con *con, kill_window_t kill_window, bool dont_kill_parent) {
+bool tree_close(Con *con, kill_window_t kill_window, bool dont_kill_parent, bool force_set_focus) {
     bool was_mapped = con->mapped;
     Con *parent = con->parent;
 
@@ -138,7 +145,7 @@ bool tree_close(Con *con, kill_window_t kill_window, bool dont_kill_parent) {
     for (child = TAILQ_FIRST(&(con->nodes_head)); child; ) {
         nextchild = TAILQ_NEXT(child, nodes);
         DLOG("killing child=%p\n", child);
-        if (!tree_close(child, kill_window, true))
+        if (!tree_close(child, kill_window, true, false))
             abort_kill = true;
         child = nextchild;
     }
@@ -191,7 +198,7 @@ bool tree_close(Con *con, kill_window_t kill_window, bool dont_kill_parent) {
     if (con_is_floating(con)) {
         Con *ws = con_get_workspace(con);
         DLOG("Container was floating, killing floating container\n");
-        tree_close(parent, DONT_KILL_WINDOW, false);
+        tree_close(parent, DONT_KILL_WINDOW, false, (con == focused));
         DLOG("parent container killed\n");
         if (con == focused) {
             DLOG("This is the focused container, i need to find another one to focus. I start looking at ws = %p\n", ws);
@@ -224,7 +231,7 @@ bool tree_close(Con *con, kill_window_t kill_window, bool dont_kill_parent) {
                 /* Instead of focusing the dockarea, we need to restore focus to the workspace */
                 con_focus(con_descend_focused(output_get_content(next->parent)));
             } else {
-                if (con != focused)
+                if (!force_set_focus && con != focused)
                     DLOG("not changing focus, the container was not focused before\n");
                 else con_focus(next);
             }
@@ -259,7 +266,7 @@ void tree_close_con(kill_window_t kill_window) {
     assert(focused->type != CT_ROOT);
 
     /* Kill con */
-    tree_close(focused, kill_window, false);
+    tree_close(focused, kill_window, false, false);
 }
 
 /*
@@ -549,7 +556,7 @@ void tree_flatten(Con *con) {
 
     /* 4: close the redundant cons */
     DLOG("closing redundant cons\n");
-    tree_close(con, DONT_KILL_WINDOW, true);
+    tree_close(con, DONT_KILL_WINDOW, true, false);
 
     /* Well, we got to abort the recursion here because we destroyed the
      * container. However, if tree_flatten() is called sufficiently often,

diff --git a/src/workspace.c b/src/workspace.c
index cf1b40705167a35791ce1e3d087c4a81505b420e..a7a5a7aad6b551b717c3408861d4f0aa0d5a190f 100644 (file)
--- a/src/workspace.c
+++ b/src/workspace.c
@@ -211,7 +211,7 @@ void workspace_show(const char *num) {
         /* check if this workspace is currently visible */
         if (!workspace_is_visible(old)) {
             LOG("Closing old workspace (%p / %s), it is empty\n", old, old->name);
-            tree_close(old, DONT_KILL_WINDOW, false);
+            tree_close(old, DONT_KILL_WINDOW, false, false);
             ipc_send_event("workspace", I3_IPC_EVENT_WORKSPACE, "{\"change\":\"empty\"}");
             changed_num_workspaces = true;
         }