Commit 860cfc039fef7e6067170e4dee224a8c1cec56af

Ran Benita 2020-04-05T00:07:57

keymap: don't forget about fallback mappings in xkb_keymap_key_get_mods_for_level() If the active set of modifiers doesn't match any explicit entry of the key type, the resulting level is 0 (i.e. Level 1). Some key types don't explicitly map Level 1, taking advantage of this fallback. Previously, xkb_keymap_key_get_mods_for_level didn't consider this, and only reported masks for explicit mappings. But this causes some glaring omissions, like matching "a" in the "us" keymap returning not results. Since every mask which isn't explicitly mapped falls back to 0, we can't return the all. Almost always the best choice for this is the empty mask, so return that, when applicable. Fixes https://github.com/xkbcommon/libxkbcommon/issues/140. Reported-by: https://github.com/AliKet Signed-off-by: Ran Benita <ran@unusedvar.com>

diff --git a/src/keymap.c b/src/keymap.c
index 54ac7c0..d2baf94 100644
--- a/src/keymap.c
+++ b/src/keymap.c
@@ -433,12 +433,39 @@ xkb_keymap_key_get_mods_for_level(struct xkb_keymap *keymap,
     const struct xkb_key_type *type = key->groups[layout].type;
 
     size_t count = 0;
-    for (unsigned i = 0; i < type->num_entries && count < masks_size; i++)
+
+    /*
+     * If the active set of modifiers doesn't match any explicit entry of
+     * the key type, the resulting level is 0 (i.e. Level 1).
+     * So, if we are asked to find the modifiers for level==0, we can offer
+     * an ~infinite supply, which is not very workable.
+     * What we do instead, is special case the empty set of modifiers for
+     * this purpose. If the empty set isn't explicit mapped to a level, we
+     * take it to map to Level 1.
+     * This is almost always what we want. If applicable, given it priority
+     * over other ways to generate the level.
+     */
+    if (level == 0) {
+        bool empty_mapped = false;
+        for (unsigned i = 0; i < type->num_entries && count < masks_size; i++)
+            if (entry_is_active(&type->entries[i]) &&
+                type->entries[i].mods.mask == 0) {
+                empty_mapped = true;
+                break;
+            }
+        if (!empty_mapped && count < masks_size) {
+            masks_out[count++] = 0;
+        }
+    }
+
+    /* Now search explicit mappings. */
+    for (unsigned i = 0; i < type->num_entries && count < masks_size; i++) {
         if (entry_is_active(&type->entries[i]) &&
             type->entries[i].level == level) {
-            masks_out[count] = type->entries[i].mods.mask;
-            ++count;
+            masks_out[count++] = type->entries[i].mods.mask;
         }
+    }
+
     return count;
 }
 
diff --git a/test/keymap.c b/test/keymap.c
index 75e59f3..a6bade8 100644
--- a/test/keymap.c
+++ b/test/keymap.c
@@ -69,7 +69,8 @@ main(void)
 
     // AC01 level 0 ('a') requires no modifiers on us-pc104
     mask_count = xkb_keymap_key_get_mods_for_level(keymap, kc, 0, 0, masks_out, 4);
-    assert(mask_count == 0);
+    assert(mask_count == 1);
+    assert(masks_out[0] == 0);
 
     shift_mask = 1 << xkb_keymap_mod_get_index(keymap, "Shift");
     lock_mask = 1 << xkb_keymap_mod_get_index(keymap, "Lock");