Merge pull request #4041 from orestisfl/floating_enable-bool

Only set FLOATING_AUTO_ON when floating_enable succeeds

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

src/floating.c

@@ -223,22 +223,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;
@@ -421,6 +421,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) {

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) {

src/manage.c

@@ -533,8 +533,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 */

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;