rcxml: call labnag with --keyboard-focus on-demand by default

The new `--keyboard-focus [none|on-demand|exclusive]` option (default:
`none`) allows to some keyboard controls in labnag:

  Right-arrow or Tab: move the button selection to the right
  Left-arrow or Shift-Tab: move the button selection to the left
  Enter: press the selected button
  Escape: close labnag

The selected button is highlighted with the inner 1px border. Maybe we can
instead use different colors for the selected button, but I prefer the
inner border for now because it doesn't require us to add new color
options or make them inherit labwc's theme.

clients/labnag.c

@@ -19,6 +19,7 @@
 #ifdef __FreeBSD__
 #include <sys/event.h> /* For signalfd() */
 #endif
+#include <sys/mman.h>
 #include <sys/signalfd.h>
 #include <sys/timerfd.h>
 #include <sys/wait.h>
@@ -26,6 +27,7 @@
 #include <unistd.h>
 #include <wayland-cursor.h>
 #include <wlr/util/log.h>
+#include <xkbcommon/xkbcommon.h>
 #include "action-prompt-codes.h"
 #include "pool-buffer.h"
 #include "cursor-shape-v1-client-protocol.h"
@@ -38,6 +40,7 @@ struct conf {
 	char *output;
 	uint32_t anchors;
 	int32_t layer; /* enum zwlr_layer_shell_v1_layer or -1 if unset */
+	enum zwlr_layer_surface_v1_keyboard_interactivity keyboard_focus;
 
 	/* Colors */
 	uint32_t button_text;
@@ -69,11 +72,18 @@ struct pointer {
 	int y;
 };
 
+struct keyboard {
+	struct wl_keyboard *keyboard;
+	struct xkb_keymap *keymap;
+	struct xkb_state *state;
+};
+
 struct seat {
 	struct wl_seat *wl_seat;
 	uint32_t wl_name;
 	struct nag *nag;
 	struct pointer pointer;
+	struct keyboard keyboard;
 	struct wl_list link; /* nag.seats */
 };
 
@@ -130,6 +140,7 @@ struct nag {
 	struct conf *conf;
 	char *message;
 	struct wl_list buttons;
+	int selected_button;
 	struct pollfd pollfds[NR_FDS];
 
 	struct {
@@ -409,7 +420,8 @@ render_detailed(cairo_t *cairo, struct nag *nag, uint32_t y)
 }
 
 static uint32_t
-render_button(cairo_t *cairo, struct nag *nag, struct button *button, int *x)
+render_button(cairo_t *cairo, struct nag *nag, struct button *button,
+		bool selected, int *x)
 {
 	int text_width, text_height;
 	get_text_size(cairo, nag->conf->font_description, &text_width,
@@ -439,6 +451,14 @@ render_button(cairo_t *cairo, struct nag *nag, struct button *button, int *x)
 			button->width, button->height);
 	cairo_fill(cairo);
 
+	if (selected) {
+		cairo_set_source_u32(cairo, nag->conf->button_border);
+		cairo_set_line_width(cairo, 1);
+		cairo_rectangle(cairo, button->x + 1.5, button->y + 1.5,
+			button->width - 3, button->height - 3);
+		cairo_stroke(cairo);
+	}
+
 	cairo_set_source_u32(cairo, nag->conf->button_text);
 	cairo_move_to(cairo, button->x + padding, button->y + padding);
 	render_text(cairo, nag->conf->font_description, 1, true,
@@ -464,11 +484,13 @@ render_to_cairo(cairo_t *cairo, struct nag *nag)
 	int x = nag->width - nag->conf->button_margin_right;
 	x -= nag->conf->button_gap_close;
 
+	int idx = 0;
 	struct button *button;
 	wl_list_for_each(button, &nag->buttons, link) {
-		h = render_button(cairo, nag, button, &x);
+		h = render_button(cairo, nag, button, idx == nag->selected_button, &x);
 		max_height = h > max_height ? h : max_height;
 		x -= nag->conf->button_gap;
+		idx++;
 	}
 
 	if (nag->details.visible) {
@@ -555,6 +577,15 @@ seat_destroy(struct seat *seat)
 	if (seat->pointer.pointer) {
 		wl_pointer_destroy(seat->pointer.pointer);
 	}
+	if (seat->keyboard.keyboard) {
+		wl_keyboard_destroy(seat->keyboard.keyboard);
+	}
+	if (seat->keyboard.keymap) {
+		xkb_keymap_unref(seat->keyboard.keymap);
+	}
+	if (seat->keyboard.state) {
+		xkb_state_unref(seat->keyboard.state);
+	}
 	wl_seat_destroy(seat->wl_seat);
 	wl_list_remove(&seat->link);
 	free(seat);
@@ -939,12 +970,170 @@ static const struct wl_pointer_listener pointer_listener = {
 	.axis_discrete = wl_pointer_axis_discrete,
 };
 
+static void
+wl_keyboard_keymap(void *data, struct wl_keyboard *wl_keyboard,
+		uint32_t format, int32_t fd, uint32_t size)
+{
+	struct seat *seat = data;
+
+	if (format != WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1) {
+		wlr_log(WLR_ERROR, "unreconizable keymap format: %d", format);
+		close(fd);
+		return;
+	}
+
+	char *map_shm = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
+	if (map_shm == MAP_FAILED) {
+		wlr_log_errno(WLR_ERROR, "mmap()");
+		close(fd);
+		return;
+	}
+
+	if (seat->keyboard.keymap) {
+		xkb_keymap_unref(seat->keyboard.keymap);
+		seat->keyboard.keymap = NULL;
+	}
+	if (seat->keyboard.state) {
+		xkb_state_unref(seat->keyboard.state);
+		seat->keyboard.state = NULL;
+	}
+	struct xkb_context *xkb = xkb_context_new(XKB_CONTEXT_NO_FLAGS);
+	seat->keyboard.keymap = xkb_keymap_new_from_string(xkb, map_shm,
+		XKB_KEYMAP_FORMAT_TEXT_V1, XKB_KEYMAP_COMPILE_NO_FLAGS);
+	if (seat->keyboard.keymap) {
+		seat->keyboard.state = xkb_state_new(seat->keyboard.keymap);
+	} else {
+		wlr_log(WLR_ERROR, "failed to compile keymap");
+	}
+	xkb_context_unref(xkb);
+
+	munmap(map_shm, size);
+	close(fd);
+}
+
+static void
+wl_keyboard_enter(void *data, struct wl_keyboard *wl_keyboard, uint32_t serial,
+		struct wl_surface *surface, struct wl_array *keys)
+{
+}
+
+static void
+wl_keyboard_leave(void *data, struct wl_keyboard *wl_keyboard, uint32_t serial,
+		struct wl_surface *surface)
+{
+}
+
+static void
+wl_keyboard_key(void *data, struct wl_keyboard *wl_keyboard, uint32_t serial,
+		uint32_t time, uint32_t key, uint32_t state)
+{
+	struct seat *seat = data;
+	struct nag *nag = seat->nag;
+
+	if (!seat->keyboard.keymap || !seat->keyboard.state) {
+		wlr_log(WLR_ERROR, "keymap/state unavailable");
+		return;
+	}
+
+	if (state != WL_KEYBOARD_KEY_STATE_PRESSED) {
+		return;
+	}
+
+	key += 8;
+	const xkb_keysym_t *syms;
+	if (!xkb_keymap_key_get_syms_by_level(seat->keyboard.keymap,
+			key, 0, 0, &syms)) {
+		wlr_log(WLR_ERROR, "failed to translate key: %d", key);
+		return;
+	}
+	xkb_mod_mask_t mods = xkb_state_serialize_mods(seat->keyboard.state,
+				XKB_STATE_MODS_EFFECTIVE);
+	xkb_mod_index_t shift_idx = xkb_keymap_mod_get_index(
+		seat->keyboard.keymap, XKB_MOD_NAME_SHIFT);
+	bool shift = shift_idx != XKB_MOD_INVALID && (mods & (1 << shift_idx));
+
+	int nr_buttons = wl_list_length(&nag->buttons);
+
+	switch (syms[0]) {
+	case XKB_KEY_Left:
+	case XKB_KEY_Right:
+	case XKB_KEY_Tab: {
+		if (nr_buttons <= 0) {
+			break;
+		}
+		int direction;
+		if (syms[0] == XKB_KEY_Left || (syms[0] == XKB_KEY_Tab && shift)) {
+			direction = 1;
+		} else {
+			direction = -1;
+		}
+		nag->selected_button += nr_buttons + direction;
+		nag->selected_button %= nr_buttons;
+		render_frame(nag);
+		close_pollfd(&nag->pollfds[FD_TIMER]);
+		break;
+	}
+	case XKB_KEY_Escape:
+		exit_status = LAB_EXIT_CANCELLED;
+		nag->run_display = false;
+		break;
+	case XKB_KEY_Return:
+	case XKB_KEY_KP_Enter: {
+		int idx = 0;
+		struct button *button;
+		wl_list_for_each(button, &nag->buttons, link) {
+			if (idx == nag->selected_button) {
+				button_execute(nag, button);
+				close_pollfd(&nag->pollfds[FD_TIMER]);
+				exit_status = idx;
+				break;
+			}
+			idx++;
+		}
+		break;
+	}
+	}
+}
+
+static void
+wl_keyboard_modifiers(void *data, struct wl_keyboard *wl_keyboard,
+		uint32_t serial, uint32_t mods_depressed, uint32_t mods_latched,
+		uint32_t mods_locked, uint32_t group)
+{
+	struct seat *seat = data;
+
+	if (!seat->keyboard.state) {
+		wlr_log(WLR_ERROR, "xkb state unavailable");
+		return;
+	}
+
+	xkb_state_update_mask(seat->keyboard.state, mods_depressed,
+		mods_latched, mods_locked, 0, 0, group);
+}
+
+static void
+wl_keyboard_repeat_info(void *data, struct wl_keyboard *wl_keyboard,
+		int32_t rate, int32_t delay)
+{
+}
+
+static const struct wl_keyboard_listener keyboard_listener = {
+	.keymap = wl_keyboard_keymap,
+	.enter = wl_keyboard_enter,
+	.leave = wl_keyboard_leave,
+	.key = wl_keyboard_key,
+	.modifiers = wl_keyboard_modifiers,
+	.repeat_info = wl_keyboard_repeat_info,
+};
+
 static void
 seat_handle_capabilities(void *data, struct wl_seat *wl_seat,
 		enum wl_seat_capability caps)
 {
 	struct seat *seat = data;
 	bool cap_pointer = caps & WL_SEAT_CAPABILITY_POINTER;
+	bool cap_keyboard = caps & WL_SEAT_CAPABILITY_KEYBOARD;
+
 	if (cap_pointer && !seat->pointer.pointer) {
 		seat->pointer.pointer = wl_seat_get_pointer(wl_seat);
 		wl_pointer_add_listener(seat->pointer.pointer,
@@ -953,6 +1142,15 @@ seat_handle_capabilities(void *data, struct wl_seat *wl_seat,
 		wl_pointer_destroy(seat->pointer.pointer);
 		seat->pointer.pointer = NULL;
 	}
+
+	if (cap_keyboard && !seat->keyboard.keyboard) {
+		seat->keyboard.keyboard = wl_seat_get_keyboard(wl_seat);
+		wl_keyboard_add_listener(seat->keyboard.keyboard,
+				&keyboard_listener, seat);
+	} else if (!cap_keyboard && seat->keyboard.keyboard) {
+		wl_keyboard_destroy(seat->keyboard.keyboard);
+		seat->keyboard.keyboard = NULL;
+	}
 }
 
 static void
@@ -1075,7 +1273,7 @@ handle_global(void *data, struct wl_registry *registry, uint32_t name,
 		}
 	} else if (strcmp(interface, zwlr_layer_shell_v1_interface.name) == 0) {
 		nag->layer_shell = wl_registry_bind(
-				registry, name, &zwlr_layer_shell_v1_interface, 1);
+				registry, name, &zwlr_layer_shell_v1_interface, 4);
 	} else if (strcmp(interface, wp_cursor_shape_manager_v1_interface.name) == 0) {
 		nag->cursor_shape_manager = wl_registry_bind(
 				registry, name, &wp_cursor_shape_manager_v1_interface, 1);
@@ -1170,6 +1368,8 @@ nag_setup(struct nag *nag)
 			&layer_surface_listener, nag);
 	zwlr_layer_surface_v1_set_anchor(nag->layer_surface,
 			nag->conf->anchors);
+	zwlr_layer_surface_v1_set_keyboard_interactivity(nag->layer_surface,
+			nag->conf->keyboard_focus);
 
 	wl_registry_destroy(registry);
 
@@ -1233,7 +1433,7 @@ nag_run(struct nag *nag)
 			wl_display_cancel_read(nag->display);
 		}
 		if (nag->pollfds[FD_TIMER].revents & POLLIN) {
-			exit_status = LAB_EXIT_TIMEOUT;
+			exit_status = LAB_EXIT_CANCELLED;
 			break;
 		}
 		if (nag->pollfds[FD_SIGNAL].revents & POLLIN) {
@@ -1250,13 +1450,7 @@ conf_init(struct conf *conf)
 		| ZWLR_LAYER_SURFACE_V1_ANCHOR_LEFT
 		| ZWLR_LAYER_SURFACE_V1_ANCHOR_RIGHT;
 	conf->layer = ZWLR_LAYER_SHELL_V1_LAYER_TOP;
-	conf->button_background = 0x333333FF;
-	conf->details_background = 0x333333FF;
-	conf->background = 0x323232FF;
-	conf->text = 0xFFFFFFFF;
-	conf->button_text = 0xFFFFFFFF;
-	conf->button_border = 0x222222FF;
-	conf->border_bottom = 0x444444FF;
+	conf->keyboard_focus = ZWLR_LAYER_SURFACE_V1_KEYBOARD_INTERACTIVITY_NONE;
 	conf->bar_border_thickness = 2;
 	conf->message_padding = 8;
 	conf->details_border_thickness = 3;
@@ -1364,6 +1558,7 @@ nag_parse_options(int argc, char **argv, struct nag *nag,
 		{"debug", no_argument, NULL, 'd'},
 		{"edge", required_argument, NULL, 'e'},
 		{"layer", required_argument, NULL, 'y'},
+		{"keyboard-focus", required_argument, NULL, 'k'},
 		{"font", required_argument, NULL, 'f'},
 		{"help", no_argument, NULL, 'h'},
 		{"detailed-message", no_argument, NULL, 'l'},
@@ -1402,6 +1597,8 @@ nag_parse_options(int argc, char **argv, struct nag *nag,
 		"  -e, --edge top|bottom           Set the edge to use.\n"
 		"  -y, --layer overlay|top|bottom|background\n"
 		"                                  Set the layer to use.\n"
+		"  -k, --keyboard-focus none|exclusive|on-demand|\n"
+		"                                  Set the policy for keyboard focus.\n"
 		"  -f, --font <font>               Set the font to use.\n"
 		"  -h, --help                      Show help message and quit.\n"
 		"  -l, --detailed-message          Read a detailed message from stdin.\n"
@@ -1433,7 +1630,7 @@ nag_parse_options(int argc, char **argv, struct nag *nag,
 
 	optind = 1;
 	while (1) {
-		int c = getopt_long(argc, argv, "B:Z:c:de:y:f:hlL:m:o:s:t:vx", opts, NULL);
+		int c = getopt_long(argc, argv, "B:Z:c:de:y:k:f:hlL:m:o:s:t:vx", opts, NULL);
 		if (c == -1) {
 			break;
 		}
@@ -1487,6 +1684,23 @@ nag_parse_options(int argc, char **argv, struct nag *nag,
 				return LAB_EXIT_FAILURE;
 			}
 			break;
+		case 'k':
+			if (strcmp(optarg, "none") == 0) {
+				conf->keyboard_focus =
+					ZWLR_LAYER_SURFACE_V1_KEYBOARD_INTERACTIVITY_NONE;
+			} else if (strcmp(optarg, "exclusive") == 0) {
+				conf->keyboard_focus =
+					ZWLR_LAYER_SURFACE_V1_KEYBOARD_INTERACTIVITY_EXCLUSIVE;
+			} else if (strcmp(optarg, "on-demand") == 0) {
+				conf->keyboard_focus =
+					ZWLR_LAYER_SURFACE_V1_KEYBOARD_INTERACTIVITY_ON_DEMAND;
+			} else {
+				fprintf(stderr, "Invalid keyboard focus: %s\n"
+						"Usage: --keyboard-focus none|exclusive|on-demand\n",
+						optarg);
+				return LAB_EXIT_FAILURE;
+			}
+			break;
 		case 'f': /* Font */
 			pango_font_description_free(conf->font_description);
 			conf->font_description = pango_font_description_from_string(optarg);
@@ -1629,6 +1843,14 @@ main(int argc, char **argv)
 		wl_list_insert(nag.buttons.prev, &nag.details.button_details->link);
 	}
 
+	int nr_buttons = wl_list_length(&nag.buttons);
+	if (conf.keyboard_focus && nr_buttons > 0) {
+		/* select the leftmost button */
+		nag.selected_button = nr_buttons - 1;
+	} else {
+		nag.selected_button = -1;
+	}
+
 	wlr_log(WLR_DEBUG, "Output: %s", nag.conf->output);
 	wlr_log(WLR_DEBUG, "Anchors: %lu", (unsigned long)nag.conf->anchors);
 	wlr_log(WLR_DEBUG, "Message: %s", nag.message);

clients/meson.build

@@ -49,6 +49,7 @@ executable(
     wlroots,
     server_protos,
     epoll_dep,
+    xkbcommon,
   ],
   include_directories: [labwc_inc],
   install: true,

docs/labnag.1.scd

@@ -31,6 +31,9 @@ _labnag_ [options...]
 *-y, --layer* overlay|top|bottom|background
 	Set the layer to use.
 
+*-k, --keyboard-focus none|exclusive|on-demand*
+	Set the policy for keyboard focus.
+
 *-f, --font* <font>
 	Set the font to use.
 

docs/labwc-config.5.scd

@@ -285,6 +285,7 @@ this is for compatibility with Openbox.
 		--button-text-color '%t' \\
 		--border-bottom-size 1 \\
 		--button-border-size 3 \\
+		--keyboard-focus on-demand \\
 		--timeout 0
 	```
 
@@ -346,7 +347,7 @@ this is for compatibility with Openbox.
 </windowSwitcher>
 ```
 
-*<windowSwitcher show="" style="" preview="" outlines="" allWorkspaces="">*
+*<windowSwitcher show="" style="" preview="" outlines="" allWorkspaces="" unshade="">*
 	*show* [yes|no] Draw the OnScreenDisplay when switching between
 	windows. Default is yes.
 
@@ -364,6 +365,9 @@ this is for compatibility with Openbox.
 	they are on. Default no (that is only windows on the current workspace
 	are shown).
 
+	*unshade* [yes|no] Temporarily unshade windows when switching between
+	them and permanently unshade on the final selection. Default is yes.
+
 *<windowSwitcher><fields><field content="" width="%">*
 	Define window switcher fields when using *<windowSwitcher style="classic" />*.
 

docs/rc.xml.all

@@ -79,7 +79,8 @@
     </font>
   </theme>
 
-  <windowSwitcher show="yes" style="classic" preview="yes" outlines="yes" allWorkspaces="no">
+  <windowSwitcher show="yes" style="classic" preview="yes"
+      outlines="yes" allWorkspaces="no" unshade="yes">
     <fields>
       <field content="icon" width="5%" />
       <field content="desktop_entry_name" width="30%" />

include/action-prompt-codes.h

@@ -3,7 +3,7 @@
 #define LABWC_ACTION_PROMPT_CODES_H
 
 #define LAB_EXIT_FAILURE 255
-#define LAB_EXIT_TIMEOUT 254
+#define LAB_EXIT_CANCELLED 254
 #define LAB_EXIT_SUCCESS 0
 
 #endif /* LABWC_ACTION_PROMPT_CODES_H */

include/config/rcxml.h

@@ -179,6 +179,7 @@ struct rcxml {
 		bool show;
 		bool preview;
 		bool outlines;
+		bool unshade;
 		enum lab_view_criteria criteria;
 		struct wl_list fields;  /* struct window_switcher_field.link */
 		enum window_switcher_style style;

include/labwc.h

@@ -303,6 +303,7 @@ struct server {
 	/* Set when in cycle (alt-tab) mode */
 	struct osd_state {
 		struct view *cycle_view;
+		bool preview_was_shaded;
 		bool preview_was_enabled;
 		struct wlr_scene_node *preview_node;
 		struct wlr_scene_tree *preview_parent;

include/view.h

@@ -106,7 +106,6 @@ struct view_size_hints {
 struct view_impl {
 	void (*configure)(struct view *view, struct wlr_box geo);
 	void (*close)(struct view *view);
-	const char *(*get_string_prop)(struct view *view, const char *prop);
 	void (*map)(struct view *view);
 	void (*set_activated)(struct view *view, bool activated);
 	void (*set_fullscreen)(struct view *view, bool fullscreen);
@@ -173,6 +172,10 @@ struct view {
 	struct wlr_scene_tree *scene_tree;
 	struct wlr_scene_tree *content_tree;
 
+	/* These are never NULL and an empty string is set instead. */
+	char *title;
+	char *app_id; /* WM_CLASS for xwayland windows */
+
 	bool mapped;
 	bool been_mapped;
 	enum lab_ssd_mode ssd_mode;
@@ -571,9 +574,8 @@ bool view_on_output(struct view *view, struct output *output);
  */
 bool view_has_strut_partial(struct view *view);
 
-const char *view_get_string_prop(struct view *view, const char *prop);
-void view_update_title(struct view *view);
-void view_update_app_id(struct view *view);
+void view_set_title(struct view *view, const char *title);
+void view_set_app_id(struct view *view, const char *app_id);
 void view_reload_ssd(struct view *view);
 
 void view_set_shade(struct view *view, bool shaded);

src/action.c

@@ -934,7 +934,7 @@ action_check_prompt_result(pid_t pid, int exit_code)
 		if (exit_code == LAB_EXIT_SUCCESS) {
 			wlr_log(WLR_INFO, "Selected the 'then' branch");
 			actions = action_get_actionlist(prompt->action, "then");
-		} else if (exit_code == LAB_EXIT_TIMEOUT) {
+		} else if (exit_code == LAB_EXIT_CANCELLED) {
 			/* no-op */
 		} else {
 			wlr_log(WLR_INFO, "Selected the 'else' branch");

src/config/rcxml.c

@@ -1217,6 +1217,8 @@ entry(xmlNode *node, char *nodename, char *content)
 			rc.window_switcher.criteria &=
 				~LAB_VIEW_CRITERIA_CURRENT_WORKSPACE;
 		}
+	} else if (!strcasecmp(nodename, "unshade.windowSwitcher")) {
+		set_bool(content, &rc.window_switcher.unshade);
 
 	/* Remove this long term - just a friendly warning for now */
 	} else if (strstr(nodename, "windowswitcher.core")) {
@@ -1429,6 +1431,7 @@ rcxml_init(void)
 	rc.window_switcher.style = WINDOW_SWITCHER_CLASSIC;
 	rc.window_switcher.preview = true;
 	rc.window_switcher.outlines = true;
+	rc.window_switcher.unshade = true;
 	rc.window_switcher.criteria = LAB_VIEW_CRITERIA_CURRENT_WORKSPACE
 		| LAB_VIEW_CRITERIA_ROOT_TOPLEVEL
 		| LAB_VIEW_CRITERIA_NO_SKIP_WINDOW_SWITCHER;
@@ -1642,6 +1645,7 @@ post_processing(void)
 				"--button-text-color '%t' "
 				"--border-bottom-size 1 "
 				"--button-border-size 3 "
+				"--keyboard-focus on-demand "
 				"--timeout 0");
 	}
 	if (!rc.fallback_app_icon_name) {

src/debug.c

@@ -71,11 +71,10 @@ get_view_part(struct view *view, struct wlr_scene_node *node)
 		return NULL;
 	}
 	if (node == &view->scene_tree->node) {
-		const char *app_id = view_get_string_prop(view, "app_id");
-		if (string_null_or_empty(app_id)) {
+		if (string_null_or_empty(view->app_id)) {
 			return "view";
 		}
-		snprintf(view_name, sizeof(view_name), "view (%s)", app_id);
+		snprintf(view_name, sizeof(view_name), "view (%s)", view->app_id);
 		return view_name;
 	}
 	if (node == &view->content_tree->node) {

src/foreign-toplevel/ext-foreign.c

@@ -32,8 +32,8 @@ handle_new_app_id(struct wl_listener *listener, void *data)
 	assert(ext_toplevel->handle);
 
 	struct wlr_ext_foreign_toplevel_handle_v1_state state = {
-		.title = view_get_string_prop(ext_toplevel->view, "title"),
-		.app_id = view_get_string_prop(ext_toplevel->view, "app_id")
+		.title = ext_toplevel->view->title,
+		.app_id = ext_toplevel->view->app_id,
 	};
 	wlr_ext_foreign_toplevel_handle_v1_update_state(ext_toplevel->handle,
 		&state);
@@ -47,8 +47,8 @@ handle_new_title(struct wl_listener *listener, void *data)
 	assert(ext_toplevel->handle);
 
 	struct wlr_ext_foreign_toplevel_handle_v1_state state = {
-		.title = view_get_string_prop(ext_toplevel->view, "title"),
-		.app_id = view_get_string_prop(ext_toplevel->view, "app_id")
+		.title = ext_toplevel->view->title,
+		.app_id = ext_toplevel->view->app_id,
 	};
 	wlr_ext_foreign_toplevel_handle_v1_update_state(ext_toplevel->handle,
 		&state);
@@ -63,15 +63,15 @@ ext_foreign_toplevel_init(struct ext_foreign_toplevel *ext_toplevel,
 	ext_toplevel->view = view;
 
 	struct wlr_ext_foreign_toplevel_handle_v1_state state = {
-		.title = view_get_string_prop(view, "title"),
-		.app_id = view_get_string_prop(view, "app_id")
+		.title = view->title,
+		.app_id = view->app_id,
 	};
 	ext_toplevel->handle = wlr_ext_foreign_toplevel_handle_v1_create(
 		view->server->foreign_toplevel_list, &state);
 
 	if (!ext_toplevel->handle) {
 		wlr_log(WLR_ERROR, "cannot create ext toplevel handle for (%s)",
-			view_get_string_prop(view, "title"));
+			view->title);
 		return;
 	}
 

src/foreign-toplevel/wlr-foreign.c

@@ -94,13 +94,8 @@ handle_new_app_id(struct wl_listener *listener, void *data)
 		wl_container_of(listener, wlr_toplevel, on_view.new_app_id);
 	assert(wlr_toplevel->handle);
 
-	const char *app_id = view_get_string_prop(wlr_toplevel->view, "app_id");
-	const char *wlr_app_id = wlr_toplevel->handle->app_id;
-	if (app_id && wlr_app_id && !strcmp(app_id, wlr_app_id)) {
-		/* Don't send app_id if they are the same */
-		return;
-	}
-	wlr_foreign_toplevel_handle_v1_set_app_id(wlr_toplevel->handle, app_id);
+	wlr_foreign_toplevel_handle_v1_set_app_id(wlr_toplevel->handle,
+		wlr_toplevel->view->app_id);
 }
 
 static void
@@ -110,13 +105,8 @@ handle_new_title(struct wl_listener *listener, void *data)
 		wl_container_of(listener, wlr_toplevel, on_view.new_title);
 	assert(wlr_toplevel->handle);
 
-	const char *title = view_get_string_prop(wlr_toplevel->view, "title");
-	const char *wlr_title = wlr_toplevel->handle->title;
-	if (title && wlr_title && !strcmp(title, wlr_title)) {
-		/* Don't send title if they are the same */
-		return;
-	}
-	wlr_foreign_toplevel_handle_v1_set_title(wlr_toplevel->handle, title);
+	wlr_foreign_toplevel_handle_v1_set_title(wlr_toplevel->handle,
+		wlr_toplevel->view->title);
 }
 
 static void
@@ -202,7 +192,7 @@ wlr_foreign_toplevel_init(struct wlr_foreign_toplevel *wlr_toplevel,
 		view->server->foreign_toplevel_manager);
 	if (!wlr_toplevel->handle) {
 		wlr_log(WLR_ERROR, "cannot create wlr foreign toplevel handle for (%s)",
-			view_get_string_prop(view, "title"));
+			view->title);
 		return;
 	}
 

src/input/keyboard.c

@@ -96,6 +96,9 @@ end_cycling(struct server *server)
 	/* FIXME: osd_finish() transiently sets focus to the old surface */
 	osd_finish(server);
 	/* Note that server->osd_state.cycle_view is cleared at this point */
+	if (rc.window_switcher.unshade) {
+		view_set_shade(cycle_view, false);
+	}
 	desktop_focus_view(cycle_view, /*raise*/ true);
 }
 

src/menu/menu.c

@@ -900,8 +900,8 @@ update_client_list_combined_menu(struct server *server)
 
 		wl_list_for_each(view, &server->views, link) {
 			if (view->workspace == workspace) {
-				const char *title = view_get_string_prop(view, "title");
-				if (!view->foreign_toplevel || string_null_or_empty(title)) {
+				if (!view->foreign_toplevel
+						|| string_null_or_empty(view->title)) {
 					continue;
 				}
 
@@ -909,9 +909,9 @@ update_client_list_combined_menu(struct server *server)
 					buf_add(&buffer, "*");
 				}
 				if (view->minimized) {
-					buf_add_fmt(&buffer, "(%s)", title);
+					buf_add_fmt(&buffer, "(%s)", view->title);
 				} else {
-					buf_add(&buffer, title);
+					buf_add(&buffer, view->title);
 				}
 				item = item_create(menu, buffer.data, NULL,
 					/*show arrow*/ false);

src/osd/osd-field.c

@@ -26,13 +26,9 @@ struct field_converter {
 /* Internal helpers */
 
 static const char *
-get_app_id_or_class(struct view *view, bool trim)
+get_identifier(struct view *view, bool trim)
 {
-	/*
-	 * XWayland clients return WM_CLASS for 'app_id' so we don't need a
-	 * special case for that here.
-	 */
-	const char *identifier = view_get_string_prop(view, "app_id");
+	const char *identifier = view->app_id;
 
 	/* remove the first two nodes of 'org.' strings */
 	if (trim && !strncmp(identifier, "org.", 4)) {
@@ -49,14 +45,13 @@ static const char *
 get_desktop_name(struct view *view)
 {
 #if HAVE_LIBSFDO
-	const char *app_id = view_get_string_prop(view, "app_id");
-	const char *name = desktop_entry_name_lookup(view->server, app_id);
+	const char *name = desktop_entry_name_lookup(view->server, view->app_id);
 	if (name) {
 		return name;
 	}
 #endif
 
-	return get_app_id_or_class(view, /* trim */ true);
+	return get_identifier(view, /* trim */ true);
 }
 
 static const char *
@@ -73,21 +68,12 @@ get_type(struct view *view, bool short_form)
 	return "???";
 }
 
-static const char *
-get_title(struct view *view)
-{
-	return view_get_string_prop(view, "title");
-}
-
 static const char *
 get_title_if_different(struct view *view)
 {
-	const char *identifier = get_app_id_or_class(view, /*trim*/ false);
-	const char *title = get_title(view);
-	if (!identifier) {
-		return title;
-	}
-	return (!title || !strcmp(identifier, title)) ? NULL : title;
+	const char *identifier = get_identifier(view, /*trim*/ false);
+	const char *title = view->title;
+	return !strcmp(identifier, title) ? NULL : title;
 }
 
 /* Field handlers */
@@ -169,14 +155,14 @@ static void
 field_set_identifier(struct buf *buf, struct view *view, const char *format)
 {
 	/* custom type conversion-specifier: I */
-	buf_add(buf, get_app_id_or_class(view, /*trim*/ false));
+	buf_add(buf, get_identifier(view, /*trim*/ false));
 }
 
 static void
 field_set_identifier_trimmed(struct buf *buf, struct view *view, const char *format)
 {
 	/* custom type conversion-specifier: i */
-	buf_add(buf, get_app_id_or_class(view, /*trim*/ true));
+	buf_add(buf, get_identifier(view, /*trim*/ true));
 }
 
 static void
@@ -190,7 +176,7 @@ static void
 field_set_title(struct buf *buf, struct view *view, const char *format)
 {
 	/* custom type conversion-specifier: T */
-	buf_add(buf, get_title(view));
+	buf_add(buf, view->title);
 }
 
 static void

src/osd/osd-thumbnail.c

@@ -155,15 +155,12 @@ create_item_scene(struct wlr_scene_tree *parent, struct view *view,
 	}
 
 	/* title */
-	const char *title = view_get_string_prop(view, "title");
-	if (title) {
-		item->normal_title = create_title(item->tree, switcher_theme,
-			title, theme->osd_label_text_color,
-			theme->osd_bg_color, title_y);
-		item->active_title = create_title(item->tree, switcher_theme,
-			title, theme->osd_label_text_color,
-			switcher_theme->item_active_bg_color, title_y);
-	}
+	item->normal_title = create_title(item->tree, switcher_theme,
+		view->title, theme->osd_label_text_color,
+		theme->osd_bg_color, title_y);
+	item->active_title = create_title(item->tree, switcher_theme,
+		view->title, theme->osd_label_text_color,
+		switcher_theme->item_active_bg_color, title_y);
 
 	/* icon */
 	int icon_size = switcher_theme->item_icon_size;

src/osd/osd.c

@@ -9,6 +9,7 @@
 #include "common/scene-helpers.h"
 #include "config/rcxml.h"
 #include "labwc.h"
+#include "node.h"
 #include "output.h"
 #include "scaled-buffer/scaled-font-buffer.h"
 #include "scaled-buffer/scaled-icon-buffer.h"
@@ -160,9 +161,14 @@ restore_preview_node(struct server *server)
 		if (!osd_state->preview_was_enabled) {
 			wlr_scene_node_set_enabled(osd_state->preview_node, false);
 		}
+		if (osd_state->preview_was_shaded) {
+			struct view *view = node_view_from_node(osd_state->preview_node);
+			view_set_shade(view, true);
+		}
 		osd_state->preview_node = NULL;
 		osd_state->preview_parent = NULL;
 		osd_state->preview_anchor = NULL;
+		osd_state->preview_was_shaded = false;
 	}
 }
 
@@ -203,6 +209,7 @@ osd_finish(struct server *server)
 	server->osd_state.preview_node = NULL;
 	server->osd_state.preview_anchor = NULL;
 	server->osd_state.cycle_view = NULL;
+	server->osd_state.preview_was_shaded = false;
 
 	destroy_osd_scenes(server);
 
@@ -244,6 +251,10 @@ preview_cycled_view(struct view *view)
 	if (!osd_state->preview_was_enabled) {
 		wlr_scene_node_set_enabled(osd_state->preview_node, true);
 	}
+	if (rc.window_switcher.unshade && view->shaded) {
+		view_set_shade(view, false);
+		osd_state->preview_was_shaded = true;
+	}
 
 	/*
 	 * FIXME: This abuses an implementation detail of the always-on-top tree.
@@ -294,6 +305,10 @@ update_osd(struct server *server)
 		}
 	}
 
+	if (rc.window_switcher.preview) {
+		preview_cycled_view(server->osd_state.cycle_view);
+	}
+
 	/* Outline current window */
 	if (rc.window_switcher.outlines) {
 		if (view_is_focusable(server->osd_state.cycle_view)) {
@@ -301,9 +316,6 @@ update_osd(struct server *server)
 		}
 	}
 
-	if (rc.window_switcher.preview) {
-		preview_cycled_view(server->osd_state.cycle_view);
-	}
 out:
 	wl_array_release(&views);
 }

src/scaled-buffer/scaled-icon-buffer.c

@@ -281,7 +281,7 @@ handle_view_new_app_id(struct wl_listener *listener, void *data)
 	struct scaled_icon_buffer *self =
 		wl_container_of(listener, self, on_view.new_app_id);
 
-	const char *app_id = view_get_string_prop(self->view, "app_id");
+	const char *app_id = self->view->app_id;
 	if (str_equal(app_id, self->view_app_id)) {
 		return;
 	}

src/ssd/ssd-titlebar.c

@@ -440,14 +440,13 @@ ssd_update_title(struct ssd *ssd)
 	}
 
 	struct view *view = ssd->view;
-	const char *title = view_get_string_prop(view, "title");
-	if (string_null_or_empty(title)) {
+	if (string_null_or_empty(view->title)) {
 		return;
 	}
 
 	struct theme *theme = view->server->theme;
 	struct ssd_state_title *state = &ssd->state.title;
-	bool title_unchanged = state->text && !strcmp(title, state->text);
+	bool title_unchanged = state->text && !strcmp(view->title, state->text);
 
 	int offset_left, offset_right;
 	get_title_offsets(ssd, &offset_left, &offset_right);
@@ -473,7 +472,7 @@ ssd_update_title(struct ssd *ssd)
 		}
 
 		const float bg_color[4] = {0, 0, 0, 0}; /* ignored */
-		scaled_font_buffer_update(subtree->title, title,
+		scaled_font_buffer_update(subtree->title, view->title,
 			title_bg_width, font,
 			text_color, bg_color);
 
@@ -483,10 +482,7 @@ ssd_update_title(struct ssd *ssd)
 	}
 
 	if (!title_unchanged) {
-		if (state->text) {
-			free(state->text);
-		}
-		state->text = xstrdup(title);
+		xstrdup_replace(state->text, view->title);
 	}
 	ssd_update_title_positions(ssd, offset_left, offset_right);
 }

src/view-impl-common.c

@@ -10,8 +10,6 @@ void
 view_impl_map(struct view *view)
 {
 	desktop_focus_view(view, /*raise*/ true);
-	view_update_title(view);
-	view_update_app_id(view);
 	if (!view->been_mapped) {
 		window_rules_apply(view, LAB_WINDOW_RULE_EVENT_ON_FIRST_MAP);
 	}
@@ -36,8 +34,7 @@ view_impl_map(struct view *view)
 	desktop_update_top_layer_visibility(view->server);
 
 	wlr_log(WLR_DEBUG, "[map] identifier=%s, title=%s",
-		view_get_string_prop(view, "app_id"),
-		view_get_string_prop(view, "title"));
+		view->app_id, view->title);
 }
 
 void

src/view.c

@@ -133,11 +133,11 @@ view_contains_window_type(struct view *view, enum lab_window_type window_type)
 bool
 view_matches_query(struct view *view, struct view_query *query)
 {
-	if (!query_str_match(query->identifier, view_get_string_prop(view, "app_id"))) {
+	if (!query_str_match(query->identifier, view->app_id)) {
 		return false;
 	}
 
-	if (!query_str_match(query->title, view_get_string_prop(view, "title"))) {
+	if (!query_str_match(query->title, view->title)) {
 		return false;
 	}
 
@@ -2385,31 +2385,36 @@ view_has_strut_partial(struct view *view)
 		view->impl->has_strut_partial(view);
 }
 
-/* Note: It is safe to assume that this function never returns NULL */
-const char *
-view_get_string_prop(struct view *view, const char *prop)
-{
-	assert(view);
-	assert(prop);
-	if (view->impl->get_string_prop) {
-		const char *ret = view->impl->get_string_prop(view, prop);
-		return ret ? ret : "";
-	}
-	return "";
-}
-
 void
-view_update_title(struct view *view)
+view_set_title(struct view *view, const char *title)
 {
 	assert(view);
+	if (!title) {
+		title = "";
+	}
+
+	if (!strcmp(view->title, title)) {
+		return;
+	}
+	xstrdup_replace(view->title, title);
+
 	ssd_update_title(view->ssd);
 	wl_signal_emit_mutable(&view->events.new_title, NULL);
 }
 
 void
-view_update_app_id(struct view *view)
+view_set_app_id(struct view *view, const char *app_id)
 {
 	assert(view);
+	if (!app_id) {
+		app_id = "";
+	}
+
+	if (!strcmp(view->app_id, app_id)) {
+		return;
+	}
+	xstrdup_replace(view->app_id, app_id);
+
 	wl_signal_emit_mutable(&view->events.new_app_id, NULL);
 }
 
@@ -2562,6 +2567,9 @@ view_init(struct view *view)
 	wl_signal_init(&view->events.activated);
 	wl_signal_init(&view->events.set_icon);
 	wl_signal_init(&view->events.destroy);
+
+	view->title = xstrdup("");
+	view->app_id = xstrdup("");
 }
 
 void
@@ -2585,6 +2593,9 @@ view_destroy(struct view *view)
 	wl_list_remove(&view->set_title.link);
 	wl_list_remove(&view->destroy.link);
 
+	zfree(view->title);
+	zfree(view->app_id);
+
 	if (view->foreign_toplevel) {
 		foreign_toplevel_destroy(view->foreign_toplevel);
 		view->foreign_toplevel = NULL;

src/xdg.c

@@ -216,7 +216,7 @@ handle_commit(struct wl_listener *listener, void *data)
 				&& extent.height == view->pending.height) {
 			wlr_log(WLR_DEBUG, "window geometry for client (%s) "
 				"appears to be incorrect - ignoring",
-				view_get_string_prop(view, "app_id"));
+				view->app_id);
 			size = extent; /* Use surface extent instead */
 		}
 	}
@@ -283,9 +283,8 @@ handle_configure_timeout(void *data)
 	assert(view->pending_configure_serial > 0);
 	assert(view->pending_configure_timeout);
 
-	const char *app_id = view_get_string_prop(view, "app_id");
 	wlr_log(WLR_INFO, "client (%s) did not respond to configure request "
-		"in %d ms", app_id, CONFIGURE_TIMEOUT_MS);
+		"in %d ms", view->app_id, CONFIGURE_TIMEOUT_MS);
 
 	wl_event_source_remove(view->pending_configure_timeout);
 	view->pending_configure_serial = 0;
@@ -492,7 +491,9 @@ static void
 handle_set_title(struct wl_listener *listener, void *data)
 {
 	struct view *view = wl_container_of(listener, view, set_title);
-	view_update_title(view);
+	struct wlr_xdg_toplevel *toplevel = xdg_toplevel_from_view(view);
+
+	view_set_title(view, toplevel->title);
 }
 
 static void
@@ -501,7 +502,9 @@ handle_set_app_id(struct wl_listener *listener, void *data)
 	struct xdg_toplevel_view *xdg_toplevel_view =
 		wl_container_of(listener, xdg_toplevel_view, set_app_id);
 	struct view *view = &xdg_toplevel_view->base;
-	view_update_app_id(view);
+	struct wlr_xdg_toplevel *toplevel = xdg_toplevel_from_view(view);
+
+	view_set_app_id(view, toplevel->app_id);
 }
 
 static void
@@ -740,31 +743,6 @@ set_initial_position(struct view *view)
 	view_place_by_policy(view, /* allow_cursor */ true, rc.placement_policy);
 }
 
-static const char *
-xdg_toplevel_view_get_string_prop(struct view *view, const char *prop)
-{
-	struct xdg_toplevel_view *xdg_view = xdg_toplevel_view_from_view(view);
-	struct wlr_xdg_toplevel *xdg_toplevel = xdg_view->xdg_surface
-		? xdg_view->xdg_surface->toplevel
-		: NULL;
-	if (!xdg_toplevel) {
-		/*
-		 * This may happen due to a matchOnce rule when
-		 * a view is destroyed while A-Tab is open. See
-		 * https://github.com/labwc/labwc/issues/1082#issuecomment-1716137180
-		 */
-		return "";
-	}
-
-	if (!strcmp(prop, "title")) {
-		return xdg_toplevel->title ? xdg_toplevel->title : "";
-	}
-	if (!strcmp(prop, "app_id")) {
-		return xdg_toplevel->app_id ? xdg_toplevel->app_id : "";
-	}
-	return "";
-}
-
 static void
 init_foreign_toplevel(struct view *view)
 {
@@ -884,7 +862,6 @@ xdg_view_get_pid(struct view *view)
 static const struct view_impl xdg_toplevel_view_impl = {
 	.configure = xdg_toplevel_view_configure,
 	.close = xdg_toplevel_view_close,
-	.get_string_prop = xdg_toplevel_view_get_string_prop,
 	.map = xdg_toplevel_view_map,
 	.set_activated = xdg_toplevel_view_set_activated,
 	.set_fullscreen = xdg_toplevel_view_set_fullscreen,

src/xwayland.c

@@ -498,7 +498,8 @@ static void
 handle_set_title(struct wl_listener *listener, void *data)
 {
 	struct view *view = wl_container_of(listener, view, set_title);
-	view_update_title(view);
+	struct xwayland_view *xwayland_view = xwayland_view_from_view(view);
+	view_set_title(view, xwayland_view->xwayland_surface->title);
 }
 
 static void
@@ -507,35 +508,7 @@ handle_set_class(struct wl_listener *listener, void *data)
 	struct xwayland_view *xwayland_view =
 		wl_container_of(listener, xwayland_view, set_class);
 	struct view *view = &xwayland_view->base;
-	view_update_app_id(view);
-}
 
-static void
-xwayland_view_close(struct view *view)
-{
-	wlr_xwayland_surface_close(xwayland_surface_from_view(view));
-}
-
-static const char *
-xwayland_view_get_string_prop(struct view *view, const char *prop)
-{
-	struct xwayland_view *xwayland_view = xwayland_view_from_view(view);
-	struct wlr_xwayland_surface *xwayland_surface = xwayland_view->xwayland_surface;
-	if (!xwayland_surface) {
-		/*
-		 * This may happen due to a matchOnce rule when
-		 * a view is destroyed while A-Tab is open. See
-		 * https://github.com/labwc/labwc/issues/1082#issuecomment-1716137180
-		 */
-		return "";
-	}
-
-	if (!strcmp(prop, "title")) {
-		return xwayland_surface->title ? xwayland_surface->title : "";
-	}
-	if (!strcmp(prop, "class")) {
-		return xwayland_surface->class ? xwayland_surface->class : "";
-	}
 	/*
 	 * Use the WM_CLASS 'instance' (1st string) for the app_id. Per
 	 * ICCCM, this is usually "the trailing part of the name used to
@@ -545,10 +518,13 @@ xwayland_view_get_string_prop(struct view *view, const char *prop)
 	 * 'instance' except for being capitalized. We want lowercase
 	 * here since we use the app_id for icon lookups.
 	 */
-	if (!strcmp(prop, "app_id")) {
-		return xwayland_surface->instance ? xwayland_surface->instance : "";
-	}
-	return "";
+	view_set_app_id(view, xwayland_view->xwayland_surface->instance);
+}
+
+static void
+xwayland_view_close(struct view *view)
+{
+	wlr_xwayland_surface_close(xwayland_surface_from_view(view));
 }
 
 static void
@@ -1050,7 +1026,6 @@ xwayland_view_get_pid(struct view *view)
 static const struct view_impl xwayland_view_impl = {
 	.configure = xwayland_view_configure,
 	.close = xwayland_view_close,
-	.get_string_prop = xwayland_view_get_string_prop,
 	.map = xwayland_view_map,
 	.set_activated = xwayland_view_set_activated,
 	.set_fullscreen = xwayland_view_set_fullscreen,