Skip to content
This repository has been archived by the owner on Apr 18, 2023. It is now read-only.

Commit

Permalink
Only set FLOATING_AUTO_ON when floating_enable succeeds
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
orestisfl committed Apr 27, 2020
1 parent 4d55bba commit 5e2f13a
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 12 deletions.
2 changes: 1 addition & 1 deletion include/floating.h
Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions src/floating.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
11 changes: 6 additions & 5 deletions src/handlers.c
Expand Up @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions src/manage.c
Expand Up @@ -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 */
Expand Down
70 changes: 70 additions & 0 deletions 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;

0 comments on commit 5e2f13a

Please sign in to comment.