osd: make window switcher more Openbox-like in terms of key precessing

Before this commit, keystrokes were interpreted based on following
hard-coded rules while the window switcher is active:

  1. Up/Left arrow keys cycle the window forward.
  2. Down/Right arrow keys cycle the window backward.
  3. Other keystrokes cycle the window in the initial direction specified
     by NextWindow/PreviousWindow actions. But while Shift key is pressed,
     the direction is inverted.

...and keybind actions were never executed.

However, this lead to a counter-intuitive behavior for new, especially
pre-Openbox users. For example, in the following keybinds, after the user
activates the window switcher with Super+n, Super+p cycles the window
_forward_:

  <keybind key="W-n">
    <action name="NextWindow" />
  </keybind>
  <keybind key="W-p">
    <action name="PreviousWindow" />
  </keybind>

This is because the key 'n' is recognized just as a normal key in the
third hard-coded rule.

So this commit changes the rules to be more Openbox-like:

  1. Up/Left arrow keys cycles the window forward.
  2. Down/Right arrow keys cycles the window backward.
  3. Other keystrokes are matched against keybinds and execute their
     actions. If they include NextWindow/PreviousWindow action, it cycles
     the selected window forward/backward even while the window switcher
     is active.

docs/labwc-actions.5.scd

@@ -117,8 +117,6 @@ Actions are used in menus and keyboard/mouse bindings.
 	Cycle focus to next/previous window respectively.++
 	Default keybind for NextWindow is Alt-Tab.
 
-	The shift key is used to reverse direction while cycling.
-
 	The arrow keys are used to move forwards/backwards while cycling.
 
 *<action name="Reconfigure" />*

include/labwc.h

@@ -387,8 +387,6 @@ struct server {
 		struct wlr_scene_tree *preview_parent;
 		struct wlr_scene_node *preview_anchor;
 		struct multi_rect *preview_outline;
-		enum lab_cycle_dir initial_direction;
-		bool initial_keybind_contained_shift;
 	} osd_state;
 
 	struct theme *theme;

src/action.c

@@ -955,10 +955,18 @@ actions_run(struct view *activator, struct server *server,
 			}
 			break;
 		case ACTION_TYPE_NEXT_WINDOW:
-			osd_begin(server, LAB_CYCLE_DIR_FORWARD);
+			if (server->input_mode == LAB_INPUT_STATE_WINDOW_SWITCHER) {
+				osd_cycle(server, LAB_CYCLE_DIR_FORWARD);
+			} else {
+				osd_begin(server, LAB_CYCLE_DIR_FORWARD);
+			}
 			break;
 		case ACTION_TYPE_PREVIOUS_WINDOW:
-			osd_begin(server, LAB_CYCLE_DIR_BACKWARD);
+			if (server->input_mode == LAB_INPUT_STATE_WINDOW_SWITCHER) {
+				osd_cycle(server, LAB_CYCLE_DIR_BACKWARD);
+			} else {
+				osd_begin(server, LAB_CYCLE_DIR_BACKWARD);
+			}
 			break;
 		case ACTION_TYPE_RECONFIGURE:
 			kill(getpid(), SIGHUP);

src/input/keyboard.c

@@ -448,58 +448,35 @@ handle_menu_keys(struct server *server, struct keysyms *syms)
 	}
 }
 
-static void
+/* Returns true if the keystroke is consumed */
-toggle_direction(enum lab_cycle_dir *direction)
+static bool
-{
-	if (*direction == LAB_CYCLE_DIR_FORWARD) {
-		*direction = LAB_CYCLE_DIR_BACKWARD;
-	} else if (*direction == LAB_CYCLE_DIR_BACKWARD) {
-		*direction = LAB_CYCLE_DIR_FORWARD;
-	}
-}
-
-static void
 handle_cycle_view_key(struct server *server, struct keyinfo *keyinfo)
 {
-	for (int i = 0; i < keyinfo->translated.nr_syms; i++) {
+	if (keyinfo->is_modifier) {
-		if (keyinfo->translated.syms[i] == XKB_KEY_Escape) {
+		return false;
-			/* cancel view-cycle */
-			osd_finish(server);
-			return;
-		}
 	}
 
 	/* cycle to next */
-	if (!keyinfo->is_modifier) {
+	for (int i = 0; i < keyinfo->translated.nr_syms; i++) {
-		enum lab_cycle_dir direction = server->osd_state.initial_direction;
+		if (keyinfo->translated.syms[i] == XKB_KEY_Escape) {
-		for (int i = 0; i < keyinfo->translated.nr_syms; i++) {
+			/* Esc deactivates window switcher */
-			if (keyinfo->translated.syms[i] == XKB_KEY_Up
+			osd_finish(server);
-					|| keyinfo->translated.syms[i] == XKB_KEY_Left) {
+			return true;
-				direction = LAB_CYCLE_DIR_BACKWARD;
-				goto miss_shift_toggle;
-			}
-			if (keyinfo->translated.syms[i] == XKB_KEY_Down
-					|| keyinfo->translated.syms[i] == XKB_KEY_Right) {
-				direction = LAB_CYCLE_DIR_FORWARD;
-				goto miss_shift_toggle;
-			}
 		}
-
+		if (keyinfo->translated.syms[i] == XKB_KEY_Up
-		bool shift_is_pressed = keyinfo->modifiers & WLR_MODIFIER_SHIFT;
+				|| keyinfo->translated.syms[i] == XKB_KEY_Left) {
-		if (shift_is_pressed != server->osd_state.initial_keybind_contained_shift) {
+			/* Up/Left cycles the window backward */
-			/*
+			osd_cycle(server, LAB_CYCLE_DIR_BACKWARD);
-			 * Shift reverses the direction - unless shift was part of the
+			return true;
-			 * original keybind in which case we do the opposite.
+		}
-			 * For example with S-A-Tab bound to PreviousWindow, shift with
+		if (keyinfo->translated.syms[i] == XKB_KEY_Down
-			 * subsequent key presses should carry on cycling backwards.
+				|| keyinfo->translated.syms[i] == XKB_KEY_Right) {
-			 */
+			/* Down/Right cycles the window forward */
-			toggle_direction(&direction);
+			osd_cycle(server, LAB_CYCLE_DIR_FORWARD);
+			return true;
 		}
-
-	/* Only one direction modifier is allowed, either arrow keys OR shift */
-miss_shift_toggle:
-		osd_cycle(server, direction);
 	}
+	return false;
 }
 
 static enum lab_key_handled
@@ -546,9 +523,10 @@ handle_compositor_keybindings(struct keyboard *keyboard,
 			handle_menu_keys(server, &keyinfo.translated);
 			return true;
 		} else if (server->input_mode == LAB_INPUT_STATE_WINDOW_SWITCHER) {
-			key_state_store_pressed_key_as_bound(event->keycode);
+			if (handle_cycle_view_key(server, &keyinfo)) {
-			handle_cycle_view_key(server, &keyinfo);
+				key_state_store_pressed_key_as_bound(event->keycode);
-			return true;
+				return true;
+			}
 		}
 	}
 

src/osd.c

@@ -160,14 +160,6 @@ restore_preview_node(struct server *server)
 	}
 }
 
-static bool
-shift_is_pressed(struct server *server)
-{
-	uint32_t modifiers = wlr_keyboard_get_modifiers(
-			&server->seat.keyboard_group->keyboard);
-	return modifiers & WLR_MODIFIER_SHIFT;
-}
-
 void
 osd_begin(struct server *server, enum lab_cycle_dir direction)
 {
@@ -175,10 +167,6 @@ osd_begin(struct server *server, enum lab_cycle_dir direction)
 		return;
 	}
 
-	/* Remember direction so it can be followed by subsequent key presses */
-	server->osd_state.initial_direction = direction;
-	server->osd_state.initial_keybind_contained_shift =
-		shift_is_pressed(server);
 	server->osd_state.cycle_view = get_next_cycle_view(server,
 		server->osd_state.cycle_view, direction);