From 0c2fbeedc2d8c11e210961c8c0ca9d43d4a044ce Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Wed, 6 Dec 2017 01:58:47 +0200 Subject: [PATCH] Don't raise floating windows when focused because of focus_follows_mouse Fixes #2990. --- include/con.h | 6 ++++ src/click.c | 4 +-- src/commands.c | 12 +++---- src/con.c | 46 +++++++++++++++++---------- src/floating.c | 6 ++-- src/handlers.c | 8 ++--- src/load_layout.c | 2 +- src/main.c | 2 +- src/manage.c | 2 +- src/move.c | 2 +- src/randr.c | 6 ++-- src/scratchpad.c | 4 +-- src/tree.c | 16 +++++----- src/workspace.c | 6 ++-- testcases/t/293-focus-follows-mouse.t | 28 ++++++++++++++++ 15 files changed, 99 insertions(+), 51 deletions(-) diff --git a/include/con.h b/include/con.h index b88ea354..726fec96 100644 --- a/include/con.h +++ b/include/con.h @@ -38,6 +38,12 @@ void con_free(Con *con); */ void con_focus(Con *con); +/** + * Sets input focus to the given container and raises it to the top. + * + */ +void con_activate(Con *con); + /** * Closes the given container. * diff --git a/src/click.c b/src/click.c index 78af8a03..5b529a38 100644 --- a/src/click.c +++ b/src/click.c @@ -241,7 +241,7 @@ static int route_click(Con *con, xcb_button_press_event_t *event, const bool mod * The splitv container will be focused. */ Con *focused = con->parent; focused = TAILQ_FIRST(&(focused->focus_head)); - con_focus(focused); + con_activate(focused); /* To prevent scrolling from going outside the container (see ticket * #557), we first check if scrolling is possible at all. */ bool scroll_prev_possible = (TAILQ_PREV(focused, nodes_head, nodes) != NULL); @@ -256,7 +256,7 @@ static int route_click(Con *con, xcb_button_press_event_t *event, const bool mod } /* 2: focus this con. */ - con_focus(con); + con_activate(con); /* 3: For floating containers, we also want to raise them on click. * We will skip handling events on floating cons in fullscreen mode */ diff --git a/src/commands.c b/src/commands.c index 162f0892..df2b236b 100644 --- a/src/commands.c +++ b/src/commands.c @@ -1261,7 +1261,7 @@ static void cmd_focus_force_focus(Con *con) { if (fullscreen_on_ws && fullscreen_on_ws != con && !con_has_parent(con, fullscreen_on_ws)) { con_disable_fullscreen(fullscreen_on_ws); } - con_focus(con); + con_activate(con); } /* @@ -1379,12 +1379,12 @@ void cmd_focus(I3_CMD) { * the target workspace, then revert focus. */ Con *currently_focused = focused; cmd_focus_force_focus(current->con); - con_focus(currently_focused); + con_activate(currently_focused); /* Now switch to the workspace, then focus */ workspace_show(ws); LOG("focusing %p / %s\n", current->con, current->con->name); - con_focus(current->con); + con_activate(current->con); count++; } @@ -1496,7 +1496,7 @@ void cmd_move_direction(I3_CMD, const char *direction, long move_px) { /* the move command should not disturb focus */ if (focused != initially_focused) - con_focus(initially_focused); + con_activate(initially_focused); // XXX: default reply for now, make this a better reply ysuccess(true); @@ -1621,7 +1621,7 @@ void cmd_open(I3_CMD) { LOG("opening new container\n"); Con *con = tree_open_con(NULL, NULL); con->layout = L_SPLITH; - con_focus(con); + con_activate(con); y(map_open); ystr("success"); @@ -2010,7 +2010,7 @@ void cmd_rename_workspace(I3_CMD, const char *old_name, const char *new_name) { } /* Restore the previous focus since con_attach messes with the focus. */ - con_focus(previously_focused); + con_activate(previously_focused); cmd_output->needs_tree_render = true; ysuccess(true); diff --git a/src/con.c b/src/con.c index c9e2b6cf..2d9b8691 100644 --- a/src/con.c +++ b/src/con.c @@ -243,15 +243,29 @@ void con_focus(Con *con) { workspace_update_urgent_flag(con_get_workspace(con)); ipc_send_window_event("urgent", con); } +} - /* Focusing a container with a floating parent should raise it to the top. Since - * con_focus is called recursively for each parent we don't need to use - * con_inside_floating(). */ - if (con->type == CT_FLOATING_CON) { - floating_raise_con(con); +/* + * Raise container to the top if it is floating or inside some floating + * container. + * + */ +static void con_raise(Con *con) { + Con *floating = con_inside_floating(con); + if (floating) { + floating_raise_con(floating); } } +/* + * Sets input focus to the given container and raises it to the top. + * + */ +void con_activate(Con *con) { + con_focus(con); + con_raise(con); +} + /* * Closes the given container. * @@ -994,9 +1008,9 @@ void con_enable_fullscreen(Con *con, fullscreen_mode_t fullscreen_mode) { Con *old_focused = focused; if (fullscreen_mode == CF_GLOBAL && cur_ws != con_ws) workspace_show(con_ws); - con_focus(con); + con_activate(con); if (fullscreen_mode != CF_GLOBAL && cur_ws != con_ws) - con_focus(old_focused); + con_activate(old_focused); con_set_fullscreen_mode(con, fullscreen_mode); } @@ -1148,11 +1162,11 @@ static bool _con_move_to_con(Con *con, Con *target, bool behind_focused, bool fi * new workspace is hidden and it's necessary to immediately switch * back to the originally-focused workspace. */ Con *old_focus = TAILQ_FIRST(&(output_get_content(dest_output)->focus_head)); - con_focus(con_descend_focused(con)); + con_activate(con_descend_focused(con)); /* Restore focus if the output's focused workspace has changed. */ if (con_get_workspace(focused) != old_focus) - con_focus(old_focus); + con_activate(old_focus); } /* 7: when moving to another workspace, we leave the focus on the current @@ -1172,7 +1186,7 @@ static bool _con_move_to_con(Con *con, Con *target, bool behind_focused, bool fi /* Set focus only if con was on current workspace before moving. * Otherwise we would give focus to some window on different workspace. */ if (!ignore_focus && source_ws == current_ws) - con_focus(con_descend_focused(focus_next)); + con_activate(con_descend_focused(focus_next)); /* 8. If anything within the container is associated with a startup sequence, * delete it so child windows won't be created on the old workspace. */ @@ -1791,7 +1805,7 @@ void con_set_layout(Con *con, layout_t layout) { con_attach(new, con, false); if (old_focused) - con_focus(old_focused); + con_activate(old_focused); tree_flatten(croot); } @@ -2358,7 +2372,7 @@ bool con_swap(Con *first, Con *second) { * We don't need to check this for the second container because we've only * moved the first one at this point.*/ if (first_ws != second_ws && focused_within_first) { - con_focus(con_descend_focused(current_ws)); + con_activate(con_descend_focused(current_ws)); } /* Move second to where first has been originally. */ @@ -2401,15 +2415,15 @@ bool con_swap(Con *first, Con *second) { */ if (focused_within_first) { if (first_ws == second_ws) { - con_focus(old_focus); + con_activate(old_focus); } else { - con_focus(con_descend_focused(second)); + con_activate(con_descend_focused(second)); } } else if (focused_within_second) { if (first_ws == second_ws) { - con_focus(old_focus); + con_activate(old_focus); } else { - con_focus(con_descend_focused(first)); + con_activate(con_descend_focused(first)); } } diff --git a/src/floating.c b/src/floating.c index 85bd8cc0..e958153d 100644 --- a/src/floating.c +++ b/src/floating.c @@ -318,7 +318,7 @@ void floating_enable(Con *con, bool automatic) { render_con(con, false); if (set_focus) - con_focus(con); + con_activate(con); /* Check if we need to re-assign it to a different workspace because of its * coordinates and exit if that was done successfully. */ @@ -382,7 +382,7 @@ void floating_disable(Con *con, bool automatic) { con_fix_percent(con->parent); if (set_focus) - con_focus(con); + con_activate(con); floating_set_hint_atom(con, false); ipc_send_window_event("floating", con); @@ -450,7 +450,7 @@ bool floating_maybe_reassign_ws(Con *con) { DLOG("Moving con %p / %s to workspace %p / %s\n", con, con->name, ws, ws->name); con_move_to_workspace(con, ws, false, true, false); workspace_show(ws); - con_focus(con_descend_focused(con)); + con_activate(con_descend_focused(con)); return true; } diff --git a/src/handlers.c b/src/handlers.c index 0f81afae..e1671c3b 100644 --- a/src/handlers.c +++ b/src/handlers.c @@ -433,7 +433,7 @@ static void handle_configure_request(xcb_configure_request_event_t *event) { if (config.focus_on_window_activation == FOWA_FOCUS || (config.focus_on_window_activation == FOWA_SMART && workspace_is_visible(ws))) { DLOG("Focusing con = %p\n", con); workspace_show(ws); - con_focus(con); + con_activate(con); tree_render(); } else if (config.focus_on_window_activation == FOWA_URGENT || (config.focus_on_window_activation == FOWA_SMART && !workspace_is_visible(ws))) { DLOG("Marking con = %p urgent\n", con); @@ -776,7 +776,7 @@ static void handle_client_message(xcb_client_message_event_t *event) { workspace_show(ws); /* Re-set focus, even if unchanged from i3’s perspective. */ focused_id = XCB_NONE; - con_focus(con); + con_activate(con); } } else { /* Request is from an application. */ @@ -788,7 +788,7 @@ static void handle_client_message(xcb_client_message_event_t *event) { if (config.focus_on_window_activation == FOWA_FOCUS || (config.focus_on_window_activation == FOWA_SMART && workspace_is_visible(ws))) { DLOG("Focusing con = %p\n", con); workspace_show(ws); - con_focus(con); + con_activate(con); } else if (config.focus_on_window_activation == FOWA_URGENT || (config.focus_on_window_activation == FOWA_SMART && !workspace_is_visible(ws))) { DLOG("Marking con = %p urgent\n", con); con_set_urgency(con, true); @@ -1245,7 +1245,7 @@ static void handle_focus_in(xcb_focus_in_event_t *event) { if (ws != con_get_workspace(focused)) workspace_show(ws); - con_focus(con); + con_activate(con); /* We update focused_id because we don’t need to set focus again */ focused_id = event->event; tree_render(); diff --git a/src/load_layout.c b/src/load_layout.c index 071b3ccd..aa7ac03c 100644 --- a/src/load_layout.c +++ b/src/load_layout.c @@ -654,6 +654,6 @@ void tree_append_json(Con *con, const char *buf, const size_t len, char **errorm yajl_free(hand); if (to_focus) { - con_focus(to_focus); + con_activate(to_focus); } } diff --git a/src/main.c b/src/main.c index b634b139..194ef05c 100644 --- a/src/main.c +++ b/src/main.c @@ -766,7 +766,7 @@ int main(int argc, char *argv[]) { output = get_first_output(); } - con_focus(con_descend_focused(output_get_content(output->con))); + con_activate(con_descend_focused(output_get_content(output->con))); free(pointerreply); } diff --git a/src/manage.c b/src/manage.c index d12ce8d6..8b306052 100644 --- a/src/manage.c +++ b/src/manage.c @@ -646,7 +646,7 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki * proper window event sequence. */ if (set_focus && nc->mapped) { DLOG("Now setting focus.\n"); - con_focus(nc); + con_activate(nc); } tree_render(); diff --git a/src/move.c b/src/move.c index 3ecc69e4..97ca6d40 100644 --- a/src/move.c +++ b/src/move.c @@ -118,7 +118,7 @@ static void move_to_output_directed(Con *con, direction_t direction) { attach_to_workspace(con, ws, direction); /* fix the focus stack */ - con_focus(con); + con_activate(con); /* force re-painting the indicators */ FREE(con->deco_render_params); diff --git a/src/randr.c b/src/randr.c index 1d333014..85add08f 100644 --- a/src/randr.c +++ b/src/randr.c @@ -496,7 +496,7 @@ void init_ws_for_output(Output *output, Con *content) { Con *ws = create_workspace_on_output(output, content); /* TODO: Set focus in main.c */ - con_focus(ws); + con_activate(ws); } /* @@ -924,7 +924,7 @@ void randr_query_outputs(void) { continue; DLOG("Focusing primary output %s\n", output_primary_name(output)); - con_focus(con_descend_focused(output->con)); + con_activate(con_descend_focused(output->con)); } /* render_layout flushes */ @@ -987,7 +987,7 @@ void randr_disable_output(Output *output) { if (next) { DLOG("now focusing next = %p\n", next); - con_focus(next); + con_activate(next); workspace_show(con_get_workspace(next)); } diff --git a/src/scratchpad.c b/src/scratchpad.c index 9018ad3f..95154014 100644 --- a/src/scratchpad.c +++ b/src/scratchpad.c @@ -123,7 +123,7 @@ void scratchpad_show(Con *con) { /* use con_descend_tiling_focused to get the last focused * window inside this scratch container in order to * keep the focus the same within this container */ - con_focus(con_descend_tiling_focused(walk_con)); + con_activate(con_descend_tiling_focused(walk_con)); return; } } @@ -205,7 +205,7 @@ void scratchpad_show(Con *con) { workspace_show(active); } - con_focus(con_descend_focused(con)); + con_activate(con_descend_focused(con)); } /* diff --git a/src/tree.c b/src/tree.c index d1c587d5..b8bc732b 100644 --- a/src/tree.c +++ b/src/tree.c @@ -344,12 +344,12 @@ bool tree_close_internal(Con *con, kill_window_t kill_window, bool dont_kill_par DLOG("focusing %p / %s\n", next, next->name); if (next->type == CT_DOCKAREA) { /* Instead of focusing the dockarea, we need to restore focus to the workspace */ - con_focus(con_descend_focused(output_get_content(next->parent))); + con_activate(con_descend_focused(output_get_content(next->parent))); } else { if (!force_set_focus && con != focused) DLOG("not changing focus, the container was not focused before\n"); else - con_focus(next); + con_activate(next); } } else { DLOG("not focusing because we're not killing anybody\n"); @@ -433,7 +433,7 @@ bool level_up(void) { /* Skip over floating containers and go directly to the grandparent * (which should always be a workspace) */ if (focused->parent->type == CT_FLOATING_CON) { - con_focus(focused->parent->parent); + con_activate(focused->parent->parent); return true; } @@ -444,7 +444,7 @@ bool level_up(void) { ELOG("'focus parent': Focus is already on the workspace, cannot go higher than that.\n"); return false; } - con_focus(focused->parent); + con_activate(focused->parent); return true; } @@ -469,7 +469,7 @@ bool level_down(void) { next = TAILQ_FIRST(&(next->focus_head)); } - con_focus(next); + con_activate(next); return true; } @@ -566,7 +566,7 @@ static bool _tree_next(Con *con, char way, orientation_t orientation, bool wrap) } workspace_show(workspace); - con_focus(focus); + con_activate(focus); x_set_warp_to(&(focus->rect)); return true; } @@ -604,7 +604,7 @@ static bool _tree_next(Con *con, char way, orientation_t orientation, bool wrap) TAILQ_INSERT_HEAD(&(parent->floating_head), last, floating_windows); } - con_focus(con_descend_focused(next)); + con_activate(con_descend_focused(next)); return true; } @@ -653,7 +653,7 @@ static bool _tree_next(Con *con, char way, orientation_t orientation, bool wrap) /* 3: focus choice comes in here. at the moment we will go down * until we find a window */ /* TODO: check for window, atm we only go down as far as possible */ - con_focus(con_descend_focused(next)); + con_activate(con_descend_focused(next)); return true; } diff --git a/src/workspace.c b/src/workspace.c index 15313357..d200b6e4 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -412,7 +412,7 @@ static void _workspace_show(Con *workspace) { if (next->urgent && (int)(config.workspace_urgency_timer * 1000) > 0) { /* focus for now… */ next->urgent = false; - con_focus(next); + con_activate(next); /* … but immediately reset urgency flags; they will be set to false by * the timer callback in case the container is focused at the time of @@ -435,7 +435,7 @@ static void _workspace_show(Con *workspace) { ev_timer_again(main_loop, focused->urgency_timer); } } else - con_focus(next); + con_activate(next); ipc_send_workspace_event("focus", workspace, current); @@ -837,7 +837,7 @@ void ws_force_orientation(Con *ws, orientation_t orientation) { con_fix_percent(ws); if (old_focused) - con_focus(old_focused); + con_activate(old_focused); } /* diff --git a/testcases/t/293-focus-follows-mouse.t b/testcases/t/293-focus-follows-mouse.t index 9fb1004e..0cd6e5c3 100644 --- a/testcases/t/293-focus-follows-mouse.t +++ b/testcases/t/293-focus-follows-mouse.t @@ -57,4 +57,32 @@ is($x->input_focus, $second->id, 'second (tabbed) window focused'); synced_warp_pointer(0, 0); is($x->input_focus, $second->id, 'second window still focused'); +################################################################### +# Test that floating windows are focused but not raised to the top. +# See issue #2990. +################################################################### + +my $ws; +my $tmp = fresh_workspace; +my ($first_floating, $second_floating); + +synced_warp_pointer(0, 0); +$first_floating = open_floating_window; +$first_floating->rect(X11::XCB::Rect->new(x => 1, y => 1, width => 100, height => 100)); +$second_floating = open_floating_window; +$second_floating->rect(X11::XCB::Rect->new(x => 50, y => 50, width => 100, height => 100)); +sync_with_i3; +$first = open_window; + +is($x->input_focus, $first->id, 'first (tiling) window focused'); +$ws = get_ws($tmp); +is($ws->{floating_nodes}->[1]->{nodes}->[0]->{window}, $second_floating->id, 'second floating on top'); +is($ws->{floating_nodes}->[0]->{nodes}->[0]->{window}, $first_floating->id, 'first floating behind'); + +synced_warp_pointer(40, 40); +is($x->input_focus, $first_floating->id, 'first floating window focused'); +$ws = get_ws($tmp); +is($ws->{floating_nodes}->[1]->{nodes}->[0]->{window}, $second_floating->id, 'second floating still on top'); +is($ws->{floating_nodes}->[0]->{nodes}->[0]->{window}, $first_floating->id, 'first floating still behind'); + done_testing;