i3 - improved tiling WM

Reviews
Bugs

Maintain the _NET_CLIENT_LIST property

Patch status: merged

Patch by Tony Crisci

Long description:

Add and update the _NET_CLIENT_LIST property on the root window to
better comply with ewmh standards.

Information on this property can be found here:
http://standards.freedesktop.org/wm-spec/latest/ar01s03.html

> These arrays contain all X Windows managed by the Window Manager.
> _NET_CLIENT_LIST has initial mapping order, starting with the oldest window.

fixes #1099

To apply this patch, use:
curl http://cr.i3wm.org/patch/502/raw.patch | git am

b/include/atoms.xmacro

@@ -13,6 +13,7 @@ xmacro(_NET_WM_WINDOW_TYPE_TOOLBAR)
 xmacro(_NET_WM_WINDOW_TYPE_SPLASH)
 xmacro(_NET_WM_DESKTOP)
 xmacro(_NET_WM_STRUT_PARTIAL)
+xmacro(_NET_CLIENT_LIST)
 xmacro(_NET_CLIENT_LIST_STACKING)
 xmacro(_NET_CURRENT_DESKTOP)
 xmacro(_NET_ACTIVE_WINDOW)

b/include/ewmh.h

@@ -28,6 +28,11 @@ void ewmh_update_current_desktop(void);
 void ewmh_update_active_window(xcb_window_t window);
 
 /**
+ * Updates the _NET_CLIENT_LIST hint. Used for window listers.
+ */
+void ewmh_update_client_list(xcb_window_t *list, int num_windows);
+
+/**
  * Updates the _NET_CLIENT_LIST_STACKING hint. Necessary to move tabs in
  * Chromium correctly.
  *

b/src/ewmh.c

@@ -72,6 +72,22 @@ void ewmh_update_workarea(void) {
 }
 
 /*
+ * Updates the _NET_CLIENT_LIST hint.
+ *
+ */
+void ewmh_update_client_list(xcb_window_t *list, int num_windows) {
+    xcb_change_property(
+        conn,
+        XCB_PROP_MODE_REPLACE,
+        root,
+        A__NET_CLIENT_LIST,
+        XCB_ATOM_WINDOW,
+        32,
+        num_windows,
+        list);
+}
+
+/*
  * Updates the _NET_CLIENT_LIST_STACKING hint.
  *
  */
@@ -122,5 +138,5 @@ void ewmh_setup_hints(void) {
     /* I’m not entirely sure if we need to keep _NET_WM_NAME on root. */
     xcb_change_property(conn, XCB_PROP_MODE_REPLACE, root, A__NET_WM_NAME, A_UTF8_STRING, 8, strlen("i3"), "i3");
 
-    xcb_change_property(conn, XCB_PROP_MODE_REPLACE, root, A__NET_SUPPORTED, XCB_ATOM_ATOM, 32, 18, supported_atoms);
+    xcb_change_property(conn, XCB_PROP_MODE_REPLACE, root, A__NET_SUPPORTED, XCB_ATOM_ATOM, 32, 19, supported_atoms);
 }

b/src/x.c

@@ -20,11 +20,6 @@ xcb_window_t focused_id = XCB_NONE;
  * tell whether the focused window actually changed. */
 static xcb_window_t last_focused = XCB_NONE;
 
-/* The bottom-to-top window stack of all windows which are managed by i3.
- * Used for x_get_window_stack(). */
-static xcb_window_t *btt_stack;
-static int btt_stack_num;
-
 /* Stores coordinates to warp mouse pointer to if set */
 static Rect *warp_to;
 
@@ -60,6 +55,7 @@ typedef struct con_state {
 
     CIRCLEQ_ENTRY(con_state) state;
     CIRCLEQ_ENTRY(con_state) old_state;
+    TAILQ_ENTRY(con_state) initial_mapping_order;
 } con_state;
 
 CIRCLEQ_HEAD(state_head, con_state) state_head =
@@ -68,6 +64,9 @@ CIRCLEQ_HEAD(state_head, con_state) state_head =
 CIRCLEQ_HEAD(old_state_head, con_state) old_state_head =
     CIRCLEQ_HEAD_INITIALIZER(old_state_head);
 
+TAILQ_HEAD(initial_mapping_head, con_state) initial_mapping_head =
+    TAILQ_HEAD_INITIALIZER(initial_mapping_head);
+
 /*
  * Returns the container state for the given frame. This function always
  * returns a container state (otherwise, there is a bug in the code and the
@@ -151,8 +150,10 @@ void x_con_init(Con *con, uint16_t depth) {
     state->id = con->frame;
     state->mapped = false;
     state->initial = true;
+    DLOG("Adding window 0x%08x to lists\n", state->id);
     CIRCLEQ_INSERT_HEAD(&state_head, state, state);
     CIRCLEQ_INSERT_HEAD(&old_state_head, state, old_state);
+    TAILQ_INSERT_TAIL(&initial_mapping_head, state, initial_mapping_order);
     DLOG("adding new state for window id 0x%08x\n", state->id);
 }
 
@@ -233,6 +234,7 @@ void x_con_kill(Con *con) {
     state = state_for_frame(con->frame);
     CIRCLEQ_REMOVE(&state_head, state, state);
     CIRCLEQ_REMOVE(&old_state_head, state, old_state);
+    TAILQ_REMOVE(&initial_mapping_head, state, initial_mapping_order);
     FREE(state->name);
     free(state);
 
@@ -909,12 +911,17 @@ void x_push_changes(Con *con) {
         if (state->con && state->con->window)
             cnt++;
 
-    if (cnt != btt_stack_num) {
-        btt_stack = srealloc(btt_stack, sizeof(xcb_window_t) * cnt);
-        btt_stack_num = cnt;
+    /* The bottom-to-top window stack of all windows which are managed by i3.
+     * Used for x_get_window_stack(). */
+    static xcb_window_t *client_list_windows = NULL;
+    static int client_list_count = 0;
+
+    if (cnt != client_list_count) {
+        client_list_windows = srealloc(client_list_windows, sizeof(xcb_window_t) * cnt);
+        client_list_count = cnt;
     }
 
-    xcb_window_t *walk = btt_stack;
+    xcb_window_t *walk = client_list_windows;
 
     /* X11 correctly represents the stack if we push it from bottom to top */
     CIRCLEQ_FOREACH_REVERSE(state, &state_head, state) {
@@ -940,9 +947,21 @@ void x_push_changes(Con *con) {
     }
 
     /* If we re-stacked something (or a new window appeared), we need to update
-     * the _NET_CLIENT_LIST_STACKING hint */
-    if (stacking_changed)
-        ewmh_update_client_list_stacking(btt_stack, btt_stack_num);
+     * the _NET_CLIENT_LIST and _NET_CLIENT_LIST_STACKING hints */
+    if (stacking_changed) {
+        DLOG("Client list changed (%i clients)\n", cnt);
+        ewmh_update_client_list_stacking(client_list_windows, client_list_count);
+
+        walk = client_list_windows;
+
+        /* reorder by initial mapping */
+        TAILQ_FOREACH(state, &initial_mapping_head, initial_mapping_order) {
+            if (state->con && state->con->window)
+                *walk++ = state->con->window->id;
+        }
+
+        ewmh_update_client_list(client_list_windows, client_list_count);
+    }
 
     DLOG("PUSHING CHANGES\n");
     x_push_node(con);

b/testcases/t/223-net-client-list.t

@@ -0,0 +1,99 @@
+#!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)
+#
+# Test that _NET_CLIENT_LIST is properly updated on the root window as windows
+# are mapped and unmapped.
+#
+# Information on this property can be found here:
+# http://standards.freedesktop.org/wm-spec/latest/ar01s03.html
+#
+# > These arrays contain all X Windows managed by the Window Manager.
+# > _NET_CLIENT_LIST has initial mapping order, starting with the oldest window.
+#
+# Ticket: #1099
+# Bug still in: 4.7.2-8-ge6cce92
+use i3test;
+
+sub get_client_list {
+    my $cookie = $x->get_property(
+        0,
+        $x->get_root_window(),
+        $x->atom(name => '_NET_CLIENT_LIST')->id,
+        $x->atom(name => 'WINDOW')->id,
+        0,
+        4096,
+    );
+    my $reply = $x->get_property_reply($cookie->{sequence});
+    my $len = $reply->{length};
+
+    return () if $len == 0;
+    return unpack("L$len", $reply->{value});
+}
+
+# Mapping a window should give us one client in _NET_CLIENT_LIST
+my $win1 = open_window;
+
+my @clients = get_client_list;
+
+is(@clients, 1, 'One client in _NET_CLIENT_LIST');
+is($clients[0], $win1->{id}, 'Correct client in position one');
+
+# Mapping another window should give us two clients in the list with the last
+# client mapped in the last position
+my $win2 = open_window;
+
+@clients = get_client_list;
+is(@clients, 2, 'Added mapped client to list (2)');
+is($clients[0], $win1->{id}, 'Correct client in position one');
+is($clients[1], $win2->{id}, 'Correct client in position two');
+
+# Mapping another window should give us three clients in the list in the order
+# they were mapped
+my $win3 = open_window;
+
+@clients = get_client_list;
+is(@clients, 3, 'Added mapped client to list (3)');
+is($clients[0], $win1->{id}, 'Correct client in position one');
+is($clients[1], $win2->{id}, 'Correct client in position two');
+is($clients[2], $win3->{id}, 'Correct client in position three');
+
+# Unmapping the second window should give us the two remaining clients in the
+# order they were mapped
+$win2->unmap;
+wait_for_unmap($win2);
+
+@clients = get_client_list;
+is(@clients, 2, 'Removed unmapped client from list (2)');
+is($clients[0], $win1->{id}, 'Correct client in position one');
+is($clients[1], $win3->{id}, 'Correct client in position two');
+
+# Unmapping the first window should give us only the remaining mapped window in
+# the list
+$win1->unmap;
+wait_for_unmap($win1);
+
+@clients = get_client_list;
+is(@clients, 1, 'Removed unmapped client from list (1)');
+is($clients[0], $win3->{id}, 'Correct client in position one');
+
+# Unmapping the last window should give us an empty list
+$win3->unmap;
+wait_for_unmap($win3);
+
+@clients = get_client_list;
+is(@clients, 0, 'Removed unmapped client from list (0)');
+
+done_testing;

191	@@ -0,0 +1,99 @@
192	+#!perl
193	+# vim:ts=4:sw=4:expandtab
194	+#
195	+# Please read the following documents before working on tests:
196	+# • http://build.i3wm.org/docs/testsuite.html
197	+# (or docs/testsuite)
198	+#
199	+# • http://build.i3wm.org/docs/lib-i3test.html
200	+# (alternatively: perldoc ./testcases/lib/i3test.pm)
201	+#
202	+# • http://build.i3wm.org/docs/ipc.html
203	+# (or docs/ipc)
204	+#
205	+# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf
206	+# (unless you are already familiar with Perl)
207	+#
208	+# Test that _NET_CLIENT_LIST is properly updated on the root window as windows
209	+# are mapped and unmapped.
210	+#
211	+# Information on this property can be found here:
212	+# http://standards.freedesktop.org/wm-spec/latest/ar01s03.html
213	+#
214	+# > These arrays contain all X Windows managed by the Window Manager.
215	+# > _NET_CLIENT_LIST has initial mapping order, starting with the oldest window.
216	+#
217	+# Ticket: #1099
218	+# Bug still in: 4.7.2-8-ge6cce92
219	+use i3test;
220	+
221	+sub get_client_list {
222	+ my $cookie = $x->get_property(
223	+ 0,
224	+ $x->get_root_window(),
225	+ $x->atom(name => '_NET_CLIENT_LIST')->id,
226	+ $x->atom(name => 'WINDOW')->id,
227	+ 0,
228	+ 4096,
229	+ );
230	+ my $reply = $x->get_property_reply($cookie->{sequence});
231	+ my $len = $reply->{length};
232	+
233	+ return () if $len == 0;
234	+ return unpack("L$len", $reply->{value});
235	+}
236	+
237	+# Mapping a window should give us one client in _NET_CLIENT_LIST
238	+my $win1 = open_window;
239	+
240	+my @clients = get_client_list;
241	+
242	+is(@clients, 1, 'One client in _NET_CLIENT_LIST');
243	+is($clients[0], $win1->{id}, 'Correct client in position one');
244	+
245	+# Mapping another window should give us two clients in the list with the last
246	+# client mapped in the last position
247	+my $win2 = open_window;
248	+
249	+@clients = get_client_list;
250	+is(@clients, 2, 'Added mapped client to list (2)');
251	+is($clients[0], $win1->{id}, 'Correct client in position one');
252	+is($clients[1], $win2->{id}, 'Correct client in position two');
253	+
254	+# Mapping another window should give us three clients in the list in the order
255	+# they were mapped
256	+my $win3 = open_window;
257	+
258	+@clients = get_client_list;
259	+is(@clients, 3, 'Added mapped client to list (3)');
260	+is($clients[0], $win1->{id}, 'Correct client in position one');
261	+is($clients[1], $win2->{id}, 'Correct client in position two');
262	+is($clients[2], $win3->{id}, 'Correct client in position three');
263	+
264	+# Unmapping the second window should give us the two remaining clients in the
265	+# order they were mapped
266	+$win2->unmap;
267	+wait_for_unmap($win2);
268	+
269	+@clients = get_client_list;
270	+is(@clients, 2, 'Removed unmapped client from list (2)');
271	+is($clients[0], $win1->{id}, 'Correct client in position one');
272	+is($clients[1], $win3->{id}, 'Correct client in position two');
273	+
274	+# Unmapping the first window should give us only the remaining mapped window in
275	+# the list
276	+$win1->unmap;
277	+wait_for_unmap($win1);
278	+
279	+@clients = get_client_list;
280	+is(@clients, 1, 'Removed unmapped client from list (1)');
281	+is($clients[0], $win3->{id}, 'Correct client in position one');
282	+
283	+# Unmapping the last window should give us an empty list
284	+$win3->unmap;
285	+wait_for_unmap($win3);
286	+
287	+@clients = get_client_list;
288	+is(@clients, 0, 'Removed unmapped client from list (0)');
289	+
290	+done_testing;

28	@@ -13,6 +13,7 @@ xmacro(_NET_WM_WINDOW_TYPE_TOOLBAR)
29	xmacro(_NET_WM_WINDOW_TYPE_SPLASH)
30	xmacro(_NET_WM_DESKTOP)
31	xmacro(_NET_WM_STRUT_PARTIAL)
32	+xmacro(_NET_CLIENT_LIST)
33	xmacro(_NET_CLIENT_LIST_STACKING)
34	xmacro(_NET_CURRENT_DESKTOP)
35	xmacro(_NET_ACTIVE_WINDOW)

40	@@ -28,6 +28,11 @@ void ewmh_update_current_desktop(void);
41	void ewmh_update_active_window(xcb_window_t window);
42
43	/**
44	+ * Updates the _NET_CLIENT_LIST hint. Used for window listers.
45	+ */
46	+void ewmh_update_client_list(xcb_window_t *list, int num_windows);
47	+
48	+/**
49	* Updates the _NET_CLIENT_LIST_STACKING hint. Necessary to move tabs in
50	* Chromium correctly.
51	*

56	@@ -72,6 +72,22 @@ void ewmh_update_workarea(void) {
57	}
58
59	/*
60	+ * Updates the _NET_CLIENT_LIST hint.
61	+ *
62	+ */
63	+void ewmh_update_client_list(xcb_window_t *list, int num_windows) {
64	+ xcb_change_property(
65	+ conn,
66	+ XCB_PROP_MODE_REPLACE,
67	+ root,
68	+ A__NET_CLIENT_LIST,
69	+ XCB_ATOM_WINDOW,
70	+ 32,
71	+ num_windows,
72	+ list);
73	+}
74	+
75	+/*
76	* Updates the _NET_CLIENT_LIST_STACKING hint.
77	*
78	*/
79	@@ -122,5 +138,5 @@ void ewmh_setup_hints(void) {
80	/* I’m not entirely sure if we need to keep _NET_WM_NAME on root. */
81	xcb_change_property(conn, XCB_PROP_MODE_REPLACE, root, A__NET_WM_NAME, A_UTF8_STRING, 8, strlen("i3"), "i3");
82
83	- xcb_change_property(conn, XCB_PROP_MODE_REPLACE, root, A__NET_SUPPORTED, XCB_ATOM_ATOM, 32, 18, supported_atoms);
84	+ xcb_change_property(conn, XCB_PROP_MODE_REPLACE, root, A__NET_SUPPORTED, XCB_ATOM_ATOM, 32, 19, supported_atoms);
85	}

90	@@ -20,11 +20,6 @@ xcb_window_t focused_id = XCB_NONE;
91	* tell whether the focused window actually changed. */
92	static xcb_window_t last_focused = XCB_NONE;
93
94	-/* The bottom-to-top window stack of all windows which are managed by i3.
95	- * Used for x_get_window_stack(). */
96	-static xcb_window_t *btt_stack;
97	-static int btt_stack_num;
98	-
99	/* Stores coordinates to warp mouse pointer to if set */
100	static Rect *warp_to;
101
102	@@ -60,6 +55,7 @@ typedef struct con_state {
103
104	CIRCLEQ_ENTRY(con_state) state;
105	CIRCLEQ_ENTRY(con_state) old_state;
106	+ TAILQ_ENTRY(con_state) initial_mapping_order;
107	} con_state;
108
109	CIRCLEQ_HEAD(state_head, con_state) state_head =
110	@@ -68,6 +64,9 @@ CIRCLEQ_HEAD(state_head, con_state) state_head =
111	CIRCLEQ_HEAD(old_state_head, con_state) old_state_head =
112	CIRCLEQ_HEAD_INITIALIZER(old_state_head);
113
114	+TAILQ_HEAD(initial_mapping_head, con_state) initial_mapping_head =
115	+ TAILQ_HEAD_INITIALIZER(initial_mapping_head);
116	+
117	/*
118	* Returns the container state for the given frame. This function always
119	* returns a container state (otherwise, there is a bug in the code and the
120	@@ -151,8 +150,10 @@ void x_con_init(Con *con, uint16_t depth) {
121	state->id = con->frame;
122	state->mapped = false;
123	state->initial = true;
124	+ DLOG("Adding window 0x%08x to lists\n", state->id);
125	CIRCLEQ_INSERT_HEAD(&state_head, state, state);
126	CIRCLEQ_INSERT_HEAD(&old_state_head, state, old_state);
127	+ TAILQ_INSERT_TAIL(&initial_mapping_head, state, initial_mapping_order);
128	DLOG("adding new state for window id 0x%08x\n", state->id);
129	}
130
131	@@ -233,6 +234,7 @@ void x_con_kill(Con *con) {
132	state = state_for_frame(con->frame);
133	CIRCLEQ_REMOVE(&state_head, state, state);
134	CIRCLEQ_REMOVE(&old_state_head, state, old_state);
135	+ TAILQ_REMOVE(&initial_mapping_head, state, initial_mapping_order);
136	FREE(state->name);
137	free(state);
138
139	@@ -909,12 +911,17 @@ void x_push_changes(Con *con) {
140	if (state->con && state->con->window)
141	cnt++;
142
143	- if (cnt != btt_stack_num) {
144	- btt_stack = srealloc(btt_stack, sizeof(xcb_window_t) * cnt);
145	- btt_stack_num = cnt;
146	+ /* The bottom-to-top window stack of all windows which are managed by i3.
147	+ * Used for x_get_window_stack(). */
148	+ static xcb_window_t *client_list_windows = NULL;
149	+ static int client_list_count = 0;
150	+
151	+ if (cnt != client_list_count) {
152	+ client_list_windows = srealloc(client_list_windows, sizeof(xcb_window_t) * cnt);
153	+ client_list_count = cnt;
154	}
155
156	- xcb_window_t *walk = btt_stack;
157	+ xcb_window_t *walk = client_list_windows;
158
159	/* X11 correctly represents the stack if we push it from bottom to top */
160	CIRCLEQ_FOREACH_REVERSE(state, &state_head, state) {
161	@@ -940,9 +947,21 @@ void x_push_changes(Con *con) {
162	}
163
164	/* If we re-stacked something (or a new window appeared), we need to update
165	- * the _NET_CLIENT_LIST_STACKING hint */
166	- if (stacking_changed)
167	- ewmh_update_client_list_stacking(btt_stack, btt_stack_num);
168	+ * the _NET_CLIENT_LIST and _NET_CLIENT_LIST_STACKING hints */
169	+ if (stacking_changed) {
170	+ DLOG("Client list changed (%i clients)\n", cnt);
171	+ ewmh_update_client_list_stacking(client_list_windows, client_list_count);
172	+
173	+ walk = client_list_windows;
174	+
175	+ /* reorder by initial mapping */
176	+ TAILQ_FOREACH(state, &initial_mapping_head, initial_mapping_order) {
177	+ if (state->con && state->con->window)
178	+ *walk++ = state->con->window->id;
179	+ }
180	+
181	+ ewmh_update_client_list(client_list_windows, client_list_count);
182	+ }
183
184	DLOG("PUSHING CHANGES\n");
185	x_push_node(con);