Set and unset individual atoms in _NET_WM_STATE instead of overwriting the entire list everytime. This allows independent management of multiple states.

fixes #1873

include/xcb.h

@@ -145,3 +145,18 @@ uint16_t get_visual_depth(xcb_visualid_t visual_id);
  *
  */
 xcb_visualid_t get_visualid_by_depth(uint16_t depth);
+
+/**
+ * Add an atom to a list of atoms the given property defines.
+ * This is useful, for example, for manipulating _NET_WM_STATE.
+ *
+ */
+void xcb_add_property_atom(xcb_connection_t *conn, xcb_window_t window, xcb_atom_t property, xcb_atom_t atom);
+
+/**
+ * Remove an atom from a list of atoms the given property defines without
+ * removing any other potentially set atoms.  This is useful, for example, for
+ * manipulating _NET_WM_STATE.
+ *
+ */
+void xcb_remove_property_atom(xcb_connection_t *conn, xcb_window_t window, xcb_atom_t property, xcb_atom_t atom);

src/con.c

@@ -658,14 +658,13 @@ static void con_set_fullscreen_mode(Con *con, fullscreen_mode_t fullscreen_mode)
     if (con->window == NULL)
         return;
 
-    uint32_t values[1];
+    if (con->fullscreen_mode != CF_NONE) {
-    unsigned int num = 0;
+        DLOG("Setting _NET_WM_STATE_FULLSCREEN for con = %p / window = %d.\n", con, con->window->id);
-
+        xcb_add_property_atom(conn, con->window->id, A__NET_WM_STATE, A__NET_WM_STATE_FULLSCREEN);
-    if (con->fullscreen_mode != CF_NONE)
+    } else {
-        values[num++] = A__NET_WM_STATE_FULLSCREEN;
+        DLOG("Removing _NET_WM_STATE_FULLSCREEN for con = %p / window = %d.\n", con, con->window->id);
-
+        xcb_remove_property_atom(conn, con->window->id, A__NET_WM_STATE, A__NET_WM_STATE_FULLSCREEN);
-    xcb_change_property(conn, XCB_PROP_MODE_REPLACE, con->window->id,
+    }
-                        A__NET_WM_STATE, XCB_ATOM_ATOM, 32, num, values);
 }
 
 /*

src/ewmh.c

@@ -218,13 +218,13 @@ void ewmh_update_client_list_stacking(xcb_window_t *stack, int num_windows) {
  *
  */
 void ewmh_update_sticky(xcb_window_t window, bool sticky) {
-    uint32_t values[1];
+    if (sticky) {
-    unsigned int num = 0;
+        DLOG("Setting _NET_WM_STATE_STICKY for window = %d.\n", window);
-
+        xcb_add_property_atom(conn, window, A__NET_WM_STATE, A__NET_WM_STATE_STICKY);
-    if (sticky)
+    } else {
-        values[num++] = A__NET_WM_STATE_STICKY;
+        DLOG("Removing _NET_WM_STATE_STICKY for window = %d.\n", window);
-
+        xcb_remove_property_atom(conn, window, A__NET_WM_STATE, A__NET_WM_STATE_STICKY);
-    xcb_change_property(conn, XCB_PROP_MODE_REPLACE, window, A__NET_WM_STATE, XCB_ATOM_ATOM, 32, num, values);
+    }
 }
 
 /*

src/x.c

@@ -631,16 +631,14 @@ static void set_hidden_state(Con *con) {
     if (should_be_hidden == state->is_hidden)
         return;
 
-    unsigned int num = 0;
-    uint32_t values[1];
     if (should_be_hidden) {
         DLOG("setting _NET_WM_STATE_HIDDEN for con = %p\n", con);
-        values[num++] = A__NET_WM_STATE_HIDDEN;
+        xcb_add_property_atom(conn, con->window->id, A__NET_WM_STATE, A__NET_WM_STATE_HIDDEN);
     } else {
         DLOG("removing _NET_WM_STATE_HIDDEN for con = %p\n", con);
+        xcb_remove_property_atom(conn, con->window->id, A__NET_WM_STATE, A__NET_WM_STATE_HIDDEN);
     }
 
-    xcb_change_property(conn, XCB_PROP_MODE_REPLACE, con->window->id, A__NET_WM_STATE, XCB_ATOM_ATOM, 32, num, values);
     state->is_hidden = should_be_hidden;
 }
 

src/xcb.c

@@ -274,3 +274,48 @@ xcb_visualid_t get_visualid_by_depth(uint16_t depth) {
     }
     return 0;
 }
+
+/*
+ * Add an atom to a list of atoms the given property defines.
+ * This is useful, for example, for manipulating _NET_WM_STATE.
+ *
+ */
+void xcb_add_property_atom(xcb_connection_t *conn, xcb_window_t window, xcb_atom_t property, xcb_atom_t atom) {
+    xcb_change_property(conn, XCB_PROP_MODE_APPEND, window, property, XCB_ATOM_ATOM, 32, 1, (uint32_t[]){atom});
+}
+
+/*
+ * Remove an atom from a list of atoms the given property defines without
+ * removing any other potentially set atoms.  This is useful, for example, for
+ * manipulating _NET_WM_STATE.
+ *
+ */
+void xcb_remove_property_atom(xcb_connection_t *conn, xcb_window_t window, xcb_atom_t property, xcb_atom_t atom) {
+    xcb_grab_server(conn);
+
+    xcb_get_property_reply_t *reply =
+        xcb_get_property_reply(conn,
+                               xcb_get_property(conn, false, window, property, XCB_GET_PROPERTY_TYPE_ANY, 0, 4096), NULL);
+    if (reply == NULL || xcb_get_property_value_length(reply) == 0)
+        goto release_grab;
+    xcb_atom_t *atoms = xcb_get_property_value(reply);
+    if (atoms == NULL) {
+        goto release_grab;
+    }
+
+    {
+        int num = 0;
+        const int current_size = xcb_get_property_value_length(reply) / (reply->format / 8);
+        xcb_atom_t values[current_size];
+        for (int i = 0; i < current_size; i++) {
+            if (atoms[i] != atom)
+                values[num++] = atoms[i];
+        }
+
+        xcb_change_property(conn, XCB_PROP_MODE_REPLACE, window, property, XCB_ATOM_ATOM, 32, num, values);
+    }
+
+release_grab:
+    FREE(reply);
+    xcb_ungrab_server(conn);
+}

testcases/t/253-multiple-net-wm-state-atoms.t

@@ -0,0 +1,70 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+#
+# Please read the following documents before working on tests:
+# • http://build.i3wm.org/docs/testsuite.html
+#   (or docs/testsuite)
+#
+# • http://build.i3wm.org/docs/lib-i3test.html
+#   (alternatively: perldoc ./testcases/lib/i3test.pm)
+#
+# • http://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)
+#
+# Ticket: #1873
+use i3test;
+use X11::XCB qw(:all);
+
+sub get_wm_state {
+    sync_with_i3;
+    my $atom = $x->atom(name => '_NET_WM_STATE_HIDDEN');
+
+    my ($con) = @_; 
+    my $cookie = $x->get_property(
+        0,  
+        $con->{id},
+        $x->atom(name => '_NET_WM_STATE')->id,
+        GET_PROPERTY_TYPE_ANY,
+        0,  
+        4096
+    );  
+
+    my $reply = $x->get_property_reply($cookie->{sequence});
+    my $len = $reply->{length};
+    return 0 if $len == 0;
+
+    my @atoms = unpack("L$len", $reply->{value});
+    return \@atoms;
+}
+
+my $wm_state_sticky = $x->atom(name => '_NET_WM_STATE_STICKY')->id;
+my $wm_state_fullscreen = $x->atom(name => '_NET_WM_STATE_FULLSCREEN')->id;
+
+##########################################################################
+# Given a sticky container, when it is fullscreened, then both wm state
+# atoms are set. When the container is unfullscreened, then only the
+# sticky atom is still set.
+##########################################################################
+
+fresh_workspace;
+my $window = open_window;
+cmd 'sticky enable';
+is_deeply(get_wm_state($window), [ $wm_state_sticky ], 'sanity check: _NET_WM_STATE_STICKY is set');
+
+cmd 'fullscreen enable';
+is_deeply(get_wm_state($window), [ $wm_state_sticky, $wm_state_fullscreen ],
+    'both _NET_WM_STATE_FULLSCREEN and _NET_WM_STATE_STICKY are set');
+
+cmd 'sticky disable';
+is_deeply(get_wm_state($window), [ $wm_state_fullscreen ], 'only _NET_WM_STATE_FULLSCREEN is set');
+
+cmd 'sticky enable';
+cmd 'fullscreen disable';
+is_deeply(get_wm_state($window), [ $wm_state_sticky ], 'only _NET_WM_STATE_STICKY is set');
+
+##########################################################################
+
+done_testing;