From 89e3ef67bcae3538e0e1ef02ad5ce42dea045763 Mon Sep 17 00:00:00 2001
From: Pierre Le Marre <dev@wismill.eu>
Date: Tue, 7 Jan 2025 16:27:50 +0100
Subject: [PATCH] state: Fix LEDs driven by the group state

When the indicator field `whichGroupState` is set to `Base` or `Latched`,
the group masks should be considered only as boolean values as per the
XKB specification.
---
 changes/api/579.bugfix.md |  2 ++
 src/state.c               | 49 +++++++++++++++++++++++++++------------
 test/state.c              |  4 ----
 3 files changed, 36 insertions(+), 19 deletions(-)
 create mode 100644 changes/api/579.bugfix.md

diff --git a/changes/api/579.bugfix.md b/changes/api/579.bugfix.md
new file mode 100644
index 000000000..368a67031
--- /dev/null
+++ b/changes/api/579.bugfix.md
@@ -0,0 +1,2 @@
+Fixed incorrect handling of group indicators when `whichGroupState` is set with
+`Base` or `Latched`.
diff --git a/src/state.c b/src/state.c
index 81dbc7666..1cd8a7b52 100644
--- a/src/state.c
+++ b/src/state.c
@@ -743,10 +743,9 @@ xkb_state_led_update_all(struct xkb_state *state)
     state->components.leds = 0;
 
     xkb_leds_enumerate(idx, led, state->keymap) {
-        xkb_mod_mask_t mod_mask = 0;
-        xkb_layout_mask_t group_mask = 0;
 
         if (led->which_mods != 0 && led->mods.mask != 0) {
+            xkb_mod_mask_t mod_mask = 0;
             if (led->which_mods & XKB_STATE_MODS_EFFECTIVE)
                 mod_mask |= state->components.mods;
             if (led->which_mods & XKB_STATE_MODS_DEPRESSED)
@@ -762,19 +761,39 @@ xkb_state_led_update_all(struct xkb_state *state)
             }
         }
 
-        if (led->which_groups != 0 && led->groups != 0) {
-            if (led->which_groups & XKB_STATE_LAYOUT_EFFECTIVE)
-                group_mask |= (1u << state->components.group);
-            if (led->which_groups & XKB_STATE_LAYOUT_DEPRESSED)
-                group_mask |= (1u << state->components.base_group);
-            if (led->which_groups & XKB_STATE_LAYOUT_LATCHED)
-                group_mask |= (1u << state->components.latched_group);
-            if (led->which_groups & XKB_STATE_LAYOUT_LOCKED)
-                group_mask |= (1u << state->components.locked_group);
-
-            if (led->groups & group_mask) {
-                state->components.leds |= (1u << idx);
-                continue;
+        if (led->which_groups != 0) {
+            if (likely(led->groups) != 0) {
+                xkb_layout_mask_t group_mask = 0;
+                /* Effective and locked groups have been brought into range */
+                assert(state->components.group < XKB_MAX_GROUPS);
+                assert(state->components.locked_group >= 0 &&
+                       state->components.locked_group < XKB_MAX_GROUPS);
+                /* Effective and locked groups are used as mask */
+                if (led->which_groups & XKB_STATE_LAYOUT_EFFECTIVE)
+                    group_mask |= (1u << state->components.group);
+                if (led->which_groups & XKB_STATE_LAYOUT_LOCKED)
+                    group_mask |= (1u << state->components.locked_group);
+                /* Base and latched groups only have to be non-zero */
+                if ((led->which_groups & XKB_STATE_LAYOUT_DEPRESSED) &&
+                    state->components.base_group != 0)
+                    group_mask |= led->groups;
+                if ((led->which_groups & XKB_STATE_LAYOUT_LATCHED) &&
+                    state->components.latched_group != 0)
+                    group_mask |= led->groups;
+
+                if (led->groups & group_mask) {
+                    state->components.leds |= (1u << idx);
+                    continue;
+                }
+            } else {
+                /* Special case for Base and latched groups */
+                if (((led->which_groups & XKB_STATE_LAYOUT_DEPRESSED) &&
+                     state->components.base_group == 0) ||
+                    ((led->which_groups & XKB_STATE_LAYOUT_LATCHED) &&
+                     state->components.latched_group == 0)) {
+                    state->components.leds |= (1u << idx);
+                    continue;
+                }
             }
         }
 
diff --git a/test/state.c b/test/state.c
index b23cfe5d0..56545780c 100644
--- a/test/state.c
+++ b/test/state.c
@@ -1204,9 +1204,7 @@ test_leds(struct xkb_context *ctx)
     const xkb_led_index_t num_idx = _xkb_keymap_led_get_index(keymap, XKB_LED_NAME_NUM);
     const xkb_led_index_t scroll_idx = _xkb_keymap_led_get_index(keymap, XKB_LED_NAME_SCROLL);
     const xkb_led_index_t compose_idx = _xkb_keymap_led_get_index(keymap, XKB_LED_NAME_COMPOSE);
-    // const xkb_led_index_t kana_idx = _xkb_keymap_led_get_index(keymap, XKB_LED_NAME_KANA);
     const xkb_led_index_t sleep_idx = _xkb_keymap_led_get_index(keymap, "Sleep");
-    // const xkb_led_index_t suspend_idx = _xkb_keymap_led_get_index(keymap, "Suspend");
     const xkb_led_index_t mute_idx = _xkb_keymap_led_get_index(keymap, "Mute");
     const xkb_led_index_t misc_idx = _xkb_keymap_led_get_index(keymap, "Misc");
     const xkb_led_index_t mail_idx = _xkb_keymap_led_get_index(keymap, "Mail");
@@ -1216,9 +1214,7 @@ test_leds(struct xkb_context *ctx)
     const xkb_led_mask_t num = 1u << num_idx;
     const xkb_led_mask_t scroll = 1u << scroll_idx;
     const xkb_led_mask_t compose = 1u << compose_idx;
-    // const xkb_led_mask_t kana = 1u << kana_idx;
     const xkb_led_mask_t sleep = 1u << sleep_idx;
-    // const xkb_led_mask_t suspend = 1u << suspend_idx;
     const xkb_led_mask_t mute = 1u << mute_idx;
     const xkb_led_mask_t misc = 1u << misc_idx;
     const xkb_led_mask_t mail = 1u << mail_idx;