From 4dbda7311480c231332a9dab9ed23d00abecb7e9 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Wed, 21 Sep 2011 23:28:01 +0100 Subject: [PATCH] Bugfix: Correctly revert focus to other floating windows when closing a floating window Uncovered by the testsuite \o/ --- include/tree.h | 9 ++++++++- src/cmdparse.y | 2 +- src/con.c | 2 +- src/floating.c | 4 ++-- src/handlers.c | 2 +- src/randr.c | 2 +- src/tree.c | 19 +++++++++++++------ src/workspace.c | 2 +- 8 files changed, 28 insertions(+), 14 deletions(-) diff --git a/include/tree.h b/include/tree.h index b9bf7f54..b483434f 100644 --- 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 9ea82efd..bf2fde96 100644 --- 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 148f7822..8fbedd3d 100644 --- 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 8d3ce4e2..b898d7bc 100644 --- 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 b3467990..336a491f 100644 --- 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 6b6cd67d..6da90070 100644 --- 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 0d0c2e9f..76530d60 100644 --- 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 cf1b4070..a7a5a7aa 100644 --- 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; } -- 2.39.5