diff --git a/include/data.h b/include/data.h index f448d263..f4dfecfe 100644 --- a/include/data.h +++ b/include/data.h @@ -241,8 +241,24 @@ struct Workspace_Assignment { TAILQ_ENTRY(Workspace_Assignment) ws_assignments; }; +typedef struct ignore_event_sequence_t { + uint32_t value; +} ignore_event_sequence_t; + +/** + * We want to prevent confusion between a uint16_t sequence and a uint32_t full sequence. + * This macro will check that the provided argument has the correct sizeof() and will + * otherwise throw an error at compile-time ("Array size is negative"). At runtime, + * the macro will cause the argument to be wrapped and returned by use of the comma + * operator expression. + * + */ +#define ENSURE_FULL_SEQUENCE(sequence) \ + ((void)sizeof(char[(sizeof(sequence) == sizeof(uint32_t)) ? 1 : -1]), \ + ((ignore_event_sequence_t){.value = sequence})) + struct Ignore_Event { - int sequence; + ignore_event_sequence_t sequence; int response_type; time_t added; diff --git a/include/handlers.h b/include/handlers.h index 18416935..12eaabad 100644 --- a/include/handlers.h +++ b/include/handlers.h @@ -14,6 +14,8 @@ #include +#include "data.h" + extern int randr_base; extern int xkb_base; extern int shape_base; @@ -26,13 +28,13 @@ extern int shape_base; * Every ignored sequence number gets garbage collected after 5 seconds. * */ -void add_ignore_event(const int sequence, const int response_type); +void add_ignore_event(const ignore_event_sequence_t sequence, const int response_type); /** * Checks if the given sequence is ignored and returns true if so. * */ -bool event_is_ignored(const int sequence, const int response_type); +bool event_is_ignored(const ignore_event_sequence_t sequence, const int response_type); /** * Takes an xcb_generic_event_t and calls the appropriate handler, based on the diff --git a/release-notes/bugfixes/3-losing-focus-when-moving-workspace b/release-notes/bugfixes/3-losing-focus-when-moving-workspace new file mode 100644 index 00000000..792450dc --- /dev/null +++ b/release-notes/bugfixes/3-losing-focus-when-moving-workspace @@ -0,0 +1 @@ +fix bug where moving a workspace to another screen would sometimes cause it to lose focus diff --git a/src/handlers.c b/src/handlers.c index 384005b9..3e1f8477 100644 --- a/src/handlers.c +++ b/src/handlers.c @@ -35,7 +35,7 @@ static SLIST_HEAD(ignore_head, Ignore_Event) ignore_events; * Every ignored sequence number gets garbage collected after 5 seconds. * */ -void add_ignore_event(const int sequence, const int response_type) { +void add_ignore_event(const ignore_event_sequence_t sequence, const int response_type) { struct Ignore_Event *event = smalloc(sizeof(struct Ignore_Event)); event->sequence = sequence; @@ -49,7 +49,7 @@ void add_ignore_event(const int sequence, const int response_type) { * Checks if the given sequence is ignored and returns true if so. * */ -bool event_is_ignored(const int sequence, const int response_type) { +bool event_is_ignored(const ignore_event_sequence_t sequence, const int response_type) { struct Ignore_Event *event; time_t now = time(NULL); for (event = SLIST_FIRST(&ignore_events); event != SLIST_END(&ignore_events);) { @@ -64,7 +64,7 @@ bool event_is_ignored(const int sequence, const int response_type) { } SLIST_FOREACH (event, &ignore_events, ignore_events) { - if (event->sequence != sequence) { + if (event->sequence.value != sequence.value) { continue; } @@ -124,7 +124,7 @@ static void check_crossing_screen_boundary(uint32_t x, uint32_t y) { * When the user moves the mouse pointer onto a window, this callback gets called. * */ -static void handle_enter_notify(xcb_enter_notify_event_t *event) { +static void handle_enter_notify(xcb_enter_notify_event_t *event, uint32_t full_sequence) { Con *con; last_timestamp = event->time; @@ -138,7 +138,7 @@ static void handle_enter_notify(xcb_enter_notify_event_t *event) { } /* Some events are not interesting, because they were not generated * actively by the user, but by reconfiguration of windows */ - if (event_is_ignored(event->sequence, XCB_ENTER_NOTIFY)) { + if (event_is_ignored(ENSURE_FULL_SEQUENCE(full_sequence), XCB_ENTER_NOTIFY)) { DLOG("Event ignored\n"); return; } @@ -280,11 +280,11 @@ static void handle_mapping_notify(xcb_mapping_notify_event_t *event) { * A new window appeared on the screen (=was mapped), so let’s manage it. * */ -static void handle_map_request(const xcb_map_request_event_t *event) { +static void handle_map_request(const xcb_map_request_event_t *event, uint32_t full_sequence) { xcb_get_window_attributes_cookie_t cookie = xcb_get_window_attributes_unchecked(conn, event->window); DLOG("window = 0x%08x, serial is %d.\n", event->window, event->sequence); - add_ignore_event(event->sequence, -1); + add_ignore_event(ENSURE_FULL_SEQUENCE(full_sequence), -1); manage_window(event->window, cookie, false); } @@ -479,7 +479,7 @@ static void handle_screen_change(xcb_generic_event_t *e) { * now, so we better clean up before. * */ -static void handle_unmap_notify_event(xcb_unmap_notify_event_t *event) { +static void handle_unmap_notify_event(xcb_unmap_notify_event_t *event, uint32_t full_sequence) { DLOG("UnmapNotify for 0x%08x (received from 0x%08x), serial %d\n", event->window, event->event, event->sequence); xcb_get_input_focus_cookie_t cookie; Con *con = con_by_window_id(event->window); @@ -532,7 +532,7 @@ ignore_end: * * Therefore, we ignore all EnterNotify events which have the same sequence * as an UnmapNotify event. */ - add_ignore_event(event->sequence, XCB_ENTER_NOTIFY); + add_ignore_event(ENSURE_FULL_SEQUENCE(full_sequence), XCB_ENTER_NOTIFY); /* Since we just ignored the sequence of this UnmapNotify, we want to make * sure that following events use a different sequence. When putting xterm @@ -551,7 +551,7 @@ ignore_end: * important fields in the event data structure). * */ -static void handle_destroy_notify_event(xcb_destroy_notify_event_t *event) { +static void handle_destroy_notify_event(xcb_destroy_notify_event_t *event, uint32_t full_sequence) { DLOG("destroy notify for 0x%08x, 0x%08x\n", event->event, event->window); xcb_unmap_notify_event_t unmap; @@ -559,7 +559,7 @@ static void handle_destroy_notify_event(xcb_destroy_notify_event_t *event) { unmap.event = event->event; unmap.window = event->window; - handle_unmap_notify_event(&unmap); + handle_unmap_notify_event(&unmap, full_sequence); } static bool window_name_changed(i3Window *window, char *old_name) { @@ -1432,11 +1432,11 @@ void handle_event(int type, xcb_generic_event_t *event) { translate_keysyms(); grab_all_keys(conn); } else if (state->xkbType == XCB_XKB_MAP_NOTIFY) { - if (event_is_ignored(event->sequence, type)) { + if (event_is_ignored(ENSURE_FULL_SEQUENCE(event->full_sequence), type)) { DLOG("Ignoring map notify event for sequence %d.\n", state->sequence); } else { DLOG("xkb map notify, sequence %d, time %d\n", state->sequence, state->time); - add_ignore_event(event->sequence, type); + add_ignore_event(ENSURE_FULL_SEQUENCE(event->full_sequence), type); xcb_key_symbols_free(keysyms); keysyms = xcb_key_symbols_alloc(conn); ungrab_all_keys(conn); @@ -1490,15 +1490,15 @@ void handle_event(int type, xcb_generic_event_t *event) { break; case XCB_MAP_REQUEST: - handle_map_request((xcb_map_request_event_t *)event); + handle_map_request((xcb_map_request_event_t *)event, event->full_sequence); break; case XCB_UNMAP_NOTIFY: - handle_unmap_notify_event((xcb_unmap_notify_event_t *)event); + handle_unmap_notify_event((xcb_unmap_notify_event_t *)event, event->full_sequence); break; case XCB_DESTROY_NOTIFY: - handle_destroy_notify_event((xcb_destroy_notify_event_t *)event); + handle_destroy_notify_event((xcb_destroy_notify_event_t *)event, event->full_sequence); break; case XCB_EXPOSE: @@ -1514,7 +1514,7 @@ void handle_event(int type, xcb_generic_event_t *event) { /* Enter window = user moved their mouse over the window */ case XCB_ENTER_NOTIFY: - handle_enter_notify((xcb_enter_notify_event_t *)event); + handle_enter_notify((xcb_enter_notify_event_t *)event, event->full_sequence); break; /* Client message are sent to the root window. The only interesting diff --git a/src/main.c b/src/main.c index 355d514e..786e4a04 100644 --- a/src/main.c +++ b/src/main.c @@ -133,7 +133,7 @@ static void xcb_prepare_cb(EV_P_ ev_prepare *w, int revents) { while ((event = xcb_poll_for_event(conn)) != NULL) { if (event->response_type == 0) { - if (event_is_ignored(event->sequence, 0)) { + if (event_is_ignored(ENSURE_FULL_SEQUENCE(event->full_sequence), 0)) { DLOG("Expected X11 Error received for sequence %x\n", event->sequence); } else { xcb_generic_error_t *error = (xcb_generic_error_t *)event; diff --git a/src/tree.c b/src/tree.c index 2ceb0688..0d505fbe 100644 --- a/src/tree.c +++ b/src/tree.c @@ -232,7 +232,7 @@ bool tree_close_internal(Con *con, kill_window_t kill_window, bool dont_kill_par /* Ignore X11 errors for the ReparentWindow request. * X11 Errors are returned when the window was already destroyed */ - add_ignore_event(cookie.sequence, 0); + add_ignore_event(ENSURE_FULL_SEQUENCE(cookie.sequence), 0); /* We are no longer handling this window, thus set WM_STATE to * WM_STATE_WITHDRAWN (see ICCCM 4.1.3.1) */ @@ -254,7 +254,7 @@ bool tree_close_internal(Con *con, kill_window_t kill_window, bool dont_kill_par /* Ignore X11 errors for the ReparentWindow request. * X11 Errors are returned when the window was already destroyed */ - add_ignore_event(cookie.sequence, 0); + add_ignore_event(ENSURE_FULL_SEQUENCE(cookie.sequence), 0); ipc_send_window_event("close", con); window_free(con->window); con->window = NULL; diff --git a/src/xcb.c b/src/xcb.c index 20bd0daa..3b37dcdf 100644 --- a/src/xcb.c +++ b/src/xcb.c @@ -112,7 +112,7 @@ void xcb_set_window_rect(xcb_connection_t *conn, const xcb_window_t window, Rect XCB_CONFIG_WINDOW_HEIGHT, &(r.x)); /* ignore events which are generated because we configured a window */ - add_ignore_event(cookie.sequence, -1); + add_ignore_event(ENSURE_FULL_SEQUENCE(cookie.sequence), -1); } /* diff --git a/testcases/t/556-workspace-keeps-focus-after-move.t b/testcases/t/556-workspace-keeps-focus-after-move.t new file mode 100644 index 00000000..467f1bef --- /dev/null +++ b/testcases/t/556-workspace-keeps-focus-after-move.t @@ -0,0 +1,60 @@ +#!perl +# vim:ts=4:sw=4:expandtab +# +# Please read the following documents before working on tests: +# • https://build.i3wm.org/docs/testsuite.html +# (or docs/testsuite) +# +# • https://build.i3wm.org/docs/lib-i3test.html +# (alternatively: perldoc ./testcases/lib/i3test.pm) +# +# • https://build.i3wm.org/docs/ipc.html +# (or docs/ipc) +# +# • https://i3wm.org/downloads/modern_perl_a4.pdf +# (unless you are already familiar with Perl) +# +# Tests that a workspace keeps its focus after moving even in event-heavy sessions. +# Ticket: #6372 +# Bug still in: 4.25-24-gf7d5b898 +use i3test i3_autostart => 0; + +my $config = <input_focus, $first->id, 'window focused'); + +# We use 'sync_with_i3' to push the sequence number for xcb events. +# The reply is a fairly cheap way of increasing the sequence number. +for (1 .. 1000) { + sync_with_i3; +} +is($x->input_focus, $first->id, 'window still focused after some events'); + +cmd 'move workspace to output right'; +is($x->input_focus, $first->id, 'window still focused after some events and after workspace has been moved'); +cmd 'move workspace to output left'; + +# We push the sequence number until it overflows a 16-bit counter. +# This triggers bugs where a uint16_t sequence is compared with a uint32_t full_sequence. +for (1 .. 1 << 16) { + sync_with_i3; +} +is($x->input_focus, $first->id, 'window still focused after many events'); + +cmd 'move workspace to output right'; +is($x->input_focus, $first->id, 'window still focused after many events and after workspace has been moved'); + +exit_gracefully($pid); + +done_testing;