Support multiple _NET_WM_STATE changes in one ClientMessage (#5910)

3 files changed, 122 insertions, 44 deletions

diff --git a/release-notes/changes/9-multiple-_NET_WM_STATE b/release-notes/changes/9-multiple-_NET_WM_STATE
new file mode 100644
index 00000000..1593056c
--- /dev/null
+++ b/release-notes/changes/9-multiple-_NET_WM_STATE
@@ -0,0 +1 @@
+support multiple _NET_WM_STATE changes in one ClientMessage
diff --git a/src/handlers.c b/src/handlers.c
index c583dade..0532e151 100644
--- a/src/handlers.c
+++ b/src/handlers.c
@@ -673,6 +673,60 @@ static void handle_expose_event(xcb_expose_event_t *event) {
 #define _NET_MOVERESIZE_WINDOW_WIDTH (1 << 10)
 #define _NET_MOVERESIZE_WINDOW_HEIGHT (1 << 11)
 
+static void handle_net_wm_state_change(Con *con, uint32_t change, uint32_t atom) {
+    if (atom == 0) {
+        return;
+    }
+
+    const char *debug_change = (change == _NET_WM_STATE_REMOVE ? "remove" : (change == _NET_WM_STATE_ADD ? "add" : "toggle"));
+
+    if (atom == A__NET_WM_STATE_FULLSCREEN) {
+        DLOG("Received a client message to %s _NET_WM_STATE_FULLSCREEN.\n", debug_change);
+
+        /* Check if the fullscreen state should be toggled */
+        if (change == _NET_WM_STATE_TOGGLE ||
+            (con->fullscreen_mode != CF_NONE && change == _NET_WM_STATE_REMOVE) ||
+            (con->fullscreen_mode == CF_NONE && change == _NET_WM_STATE_ADD)) {
+            DLOG("toggling fullscreen\n");
+            con_toggle_fullscreen(con, CF_OUTPUT);
+        }
+    } else if (atom == A__NET_WM_STATE_DEMANDS_ATTENTION) {
+        DLOG("Received a client message to %s _NET_WM_STATE_DEMANDS_ATTENTION.\n", debug_change);
+
+        /* Check if the urgent flag must be set or not */
+        if (change == _NET_WM_STATE_ADD) {
+            con_set_urgency(con, true);
+            con = remanage_window(con);
+        } else if (change == _NET_WM_STATE_REMOVE) {
+            con_set_urgency(con, false);
+            con = remanage_window(con);
+        } else if (change == _NET_WM_STATE_TOGGLE) {
+            con_set_urgency(con, !con->urgent);
+            con = remanage_window(con);
+        }
+    } else if (atom == A__NET_WM_STATE_STICKY) {
+        DLOG("Received a client message to %s _NET_WM_STATE_STICKY.\n", debug_change);
+
+        if (change == _NET_WM_STATE_ADD) {
+            con->sticky = true;
+        } else if (change == _NET_WM_STATE_REMOVE) {
+            con->sticky = false;
+        } else if (change == _NET_WM_STATE_TOGGLE) {
+            con->sticky = !con->sticky;
+        }
+
+        DLOG("New sticky status for con = %p is %i.\n", con, con->sticky);
+        ewmh_update_sticky(con->window->id, con->sticky);
+        output_push_sticky_windows(focused);
+        ewmh_update_wm_desktop();
+    } else {
+        DLOG("Unknown atom in ClientMessage to %s type %u\n", debug_change, atom);
+        return;
+    }
+
+    tree_render();
+}
+
 /*
  * Handle client messages (EWMH)
  *
@@ -686,11 +740,8 @@ static void handle_client_message(xcb_client_message_event_t *event) {
 
     LOG("ClientMessage for window 0x%08x\n", event->window);
     if (event->type == A__NET_WM_STATE) {
-        if (event->format != 32 ||
-            (event->data.data32[1] != A__NET_WM_STATE_FULLSCREEN &&
-             event->data.data32[1] != A__NET_WM_STATE_DEMANDS_ATTENTION &&
-             event->data.data32[1] != A__NET_WM_STATE_STICKY)) {
-            DLOG("Unknown atom in clientmessage of type %d\n", event->data.data32[1]);
+        if (event->format != 32) {
+            DLOG("Unknown format %d in ClientMessage\n", event->format);
             return;
         }
 
@@ -700,46 +751,9 @@ static void handle_client_message(xcb_client_message_event_t *event) {
             return;
         }
 
-        if (event->data.data32[1] == A__NET_WM_STATE_FULLSCREEN) {
-            /* Check if the fullscreen state should be toggled */
-            if ((con->fullscreen_mode != CF_NONE &&
-                 (event->data.data32[0] == _NET_WM_STATE_REMOVE ||
-                  event->data.data32[0] == _NET_WM_STATE_TOGGLE)) ||
-                (con->fullscreen_mode == CF_NONE &&
-                 (event->data.data32[0] == _NET_WM_STATE_ADD ||
-                  event->data.data32[0] == _NET_WM_STATE_TOGGLE))) {
-                DLOG("toggling fullscreen\n");
-                con_toggle_fullscreen(con, CF_OUTPUT);
-            }
-        } else if (event->data.data32[1] == A__NET_WM_STATE_DEMANDS_ATTENTION) {
-            /* Check if the urgent flag must be set or not */
-            if (event->data.data32[0] == _NET_WM_STATE_ADD) {
-                con_set_urgency(con, true);
-                con = remanage_window(con);
-            } else if (event->data.data32[0] == _NET_WM_STATE_REMOVE) {
-                con_set_urgency(con, false);
-                con = remanage_window(con);
-            } else if (event->data.data32[0] == _NET_WM_STATE_TOGGLE) {
-                con_set_urgency(con, !con->urgent);
-                con = remanage_window(con);
-            }
-        } else if (event->data.data32[1] == A__NET_WM_STATE_STICKY) {
-            DLOG("Received a client message to modify _NET_WM_STATE_STICKY.\n");
-            if (event->data.data32[0] == _NET_WM_STATE_ADD) {
-                con->sticky = true;
-            } else if (event->data.data32[0] == _NET_WM_STATE_REMOVE) {
-                con->sticky = false;
-            } else if (event->data.data32[0] == _NET_WM_STATE_TOGGLE) {
-                con->sticky = !con->sticky;
-            }
-
-            DLOG("New sticky status for con = %p is %i.\n", con, con->sticky);
-            ewmh_update_sticky(con->window->id, con->sticky);
-            output_push_sticky_windows(focused);
-            ewmh_update_wm_desktop();
+        for (size_t i = 0; i < sizeof(event->data.data32) / sizeof(event->data.data32[0]) - 1; i++) {
+            handle_net_wm_state_change(con, event->data.data32[0], event->data.data32[i + 1]);
         }
-
-        tree_render();
     } else if (event->type == A__NET_ACTIVE_WINDOW) {
         if (event->format != 32) {
             return;
diff --git a/testcases/t/552-net-wm-state-multiple-changes.t b/testcases/t/552-net-wm-state-multiple-changes.t
new file mode 100644
index 00000000..136ca1af
--- /dev/null
+++ b/testcases/t/552-net-wm-state-multiple-changes.t
@@ -0,0 +1,63 @@
+#!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)
+#
+# Test that i3 supports setting multiple _NET_WM_STATE changes in one ClientMessage.
+# Bug still in: 4.23-21-g6a530de2
+use i3test;
+
+my $_NET_WM_STATE_REMOVE = 0;
+my $_NET_WM_STATE_ADD = 1;
+my $_NET_WM_STATE_TOGGLE = 2;
+sub send_event {
+    my ($win, $add) = @_;
+    my $msg = pack "CCSLLLLLL",
+        X11::XCB::CLIENT_MESSAGE, # response_type
+        32, # format
+        0, # sequence
+        $win->id, # window
+        $x->atom(name => '_NET_WM_STATE')->id, # message type
+        ($add ? $_NET_WM_STATE_ADD : $_NET_WM_STATE_REMOVE), # data32[0]
+        $x->atom(name => '_NET_WM_STATE_FULLSCREEN')->id, # data32[1]
+        $x->atom(name => '_NET_WM_STATE_STICKY')->id, # data32[2]
+        0, # data32[3]
+        0; # data32[4]
+
+    $x->send_event(0, $x->get_root_window(), X11::XCB::EVENT_MASK_SUBSTRUCTURE_REDIRECT, $msg);
+    sync_with_i3;
+}
+
+my $ws1 = fresh_workspace;
+my $win = open_floating_window;
+
+# Nothing to remove
+send_event($win, 0);
+is_num_fullscreen($ws1, 0, 'no fullscreen window');
+
+# Enable
+send_event($win, 1);
+is_num_fullscreen($ws1, 1, 'one fullscreen window');
+my $ws2 = fresh_workspace;
+is_num_fullscreen($ws2, 1, 'sticky fullscreen window in second workspace');
+
+# Disable
+send_event($win, 0);
+is_num_fullscreen($ws1, 0, 'no fullscreen windows');
+is_num_fullscreen($ws2, 0, 'no fullscreen windows');
+cmd "workspace $ws1";
+is(@{get_ws($ws1)->{floating_nodes}}, 0, 'No floating (sticky) window in first workspace');
+is(@{get_ws($ws2)->{floating_nodes}}, 1, 'One floating (non-sticky) window in second workspace');
+
+done_testing;