From 5e2f13a28c4fd7348914dfd999026d0e30424cd5 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Mon, 27 Apr 2020 13:10:15 +0200 Subject: [PATCH] Only set FLOATING_AUTO_ON when floating_enable succeeds Fixes #4039 Crash with docking clients where the floating field is set even though floating_enable refuses to make them floating. See issue for example with logs. --- include/floating.h | 2 +- src/floating.c | 9 ++-- src/handlers.c | 11 ++-- src/manage.c | 5 +- testcases/t/310-client-message-sticky.t | 70 +++++++++++++++++++++++++ 5 files changed, 85 insertions(+), 12 deletions(-) create mode 100644 testcases/t/310-client-message-sticky.t diff --git a/include/floating.h b/include/floating.h index 8e60f78c7..612874fcf 100644 --- a/include/floating.h +++ b/include/floating.h @@ -25,7 +25,7 @@ typedef enum { BORDER_LEFT = (1 << 0), * floating_windows list of the workspace. * */ -void floating_enable(Con *con, bool automatic); +bool floating_enable(Con *con, bool automatic); /** * Disables floating mode for the given container by re-attaching the container diff --git a/src/floating.c b/src/floating.c index 4fed43e44..7302f6447 100644 --- a/src/floating.c +++ b/src/floating.c @@ -221,22 +221,22 @@ void floating_check_size(Con *floating_con, bool prefer_height) { } } -void floating_enable(Con *con, bool automatic) { +bool floating_enable(Con *con, bool automatic) { bool set_focus = (con == focused); if (con_is_docked(con)) { LOG("Container is a dock window, not enabling floating mode.\n"); - return; + return false; } if (con_is_floating(con)) { LOG("Container is already in floating mode, not doing anything.\n"); - return; + return false; } if (con->type == CT_WORKSPACE) { LOG("Container is a workspace, not enabling floating mode.\n"); - return; + return false; } Con *focus_head_placeholder = NULL; @@ -419,6 +419,7 @@ void floating_enable(Con *con, bool automatic) { floating_set_hint_atom(nc, true); ipc_send_window_event("floating", con); + return true; } void floating_disable(Con *con) { diff --git a/src/handlers.c b/src/handlers.c index d6d6e20bc..f54489545 100644 --- a/src/handlers.c +++ b/src/handlers.c @@ -841,12 +841,13 @@ static void handle_client_message(xcb_client_message_event_t *event) { * let's float it and make it sticky. */ DLOG("The window was requested to be visible on all workspaces, making it sticky and floating.\n"); - floating_enable(con, false); - con->floating = FLOATING_AUTO_ON; + if (floating_enable(con, false)) { + con->floating = FLOATING_AUTO_ON; - con->sticky = true; - ewmh_update_sticky(con->window->id, true); - output_push_sticky_windows(focused); + con->sticky = true; + ewmh_update_sticky(con->window->id, true); + output_push_sticky_windows(focused); + } } else { Con *ws = ewmh_get_workspace_by_index(index); if (ws == NULL) { diff --git a/src/manage.c b/src/manage.c index eecba95c5..19ce87755 100644 --- a/src/manage.c +++ b/src/manage.c @@ -537,8 +537,9 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki * was not specified */ bool automatic_border = (motif_border_style == BS_NORMAL); - floating_enable(nc, automatic_border); - nc->floating = FLOATING_AUTO_ON; + if (floating_enable(nc, automatic_border)) { + nc->floating = FLOATING_AUTO_ON; + } } /* explicitly set the border width to the default */ diff --git a/testcases/t/310-client-message-sticky.t b/testcases/t/310-client-message-sticky.t new file mode 100644 index 000000000..0e7d8b7c3 --- /dev/null +++ b/testcases/t/310-client-message-sticky.t @@ -0,0 +1,70 @@ +#!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) +# +# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf +# (unless you are already familiar with Perl) +# +# Verify that _NET_WM_DESKTOP sticky requests do not conflict with dock +# clients, resulting in a crash +# Ticket: #4039 +# Bug still in: 4.18-238-g4d55bba7f +use i3test; + +sub send_sticky_request { + my ($win) = @_; + + my $msg = pack "CCSLLLLLL", + X11::XCB::CLIENT_MESSAGE, # response_type + 32, # format + 0, # sequence + $win->id, # window + $x->atom(name => '_NET_WM_DESKTOP')->id, # message type + hex '0xFFFFFFFF', # data32[0] = NET_WM_DESKTOP_ALL + 0, # data32[1] + 0, # data32[2] + 0, # data32[3] + 0; # data32[4] + + $x->send_event(0, $x->get_root_window(), X11::XCB::EVENT_MASK_SUBSTRUCTURE_REDIRECT, $msg); +} + +# Test the normal functionality first +my $ws = fresh_workspace; +my $win = open_window; + +is(@{get_ws($ws)->{floating_nodes}}, 0, 'No floating windows yet'); +send_sticky_request($win); +sync_with_i3; + +is(@{get_ws($ws)->{floating_nodes}}, 1, 'One floating (sticky) window'); +$ws = fresh_workspace; +is(@{get_ws($ws)->{floating_nodes}}, 1, 'Sticky window in new workspace'); + +# See #4039 +kill_all_windows; +$win = open_window({ + window_type => $x->atom(name => '_NET_WM_WINDOW_TYPE_DOCK') +}); + +send_sticky_request($win); +sync_with_i3; +is(@{get_ws($ws)->{floating_nodes}}, 0, 'Dock client did not get sticky/floating'); + +# Cause a ConfigureRequest by setting the window’s position/size. +my ($a, $t) = $win->rect; +$win->rect(X11::XCB::Rect->new(x => 0, y => 0, width => $a->width, height => $a->height)); + +does_i3_live; +is(@{get_ws($ws)->{floating_nodes}}, 0, 'Dock client still not sticky/floating'); + +done_testing;