Commit 5417440970f15673fbc128108245ad0697fcadc0
2014-03-27T17:42:20
state: fix consumed modifier calculation The current calculation is in short: entry ? (entry->mask & ~entry->preserve) : 0 This changes it be type->mask & ~(entry ? entry->preserve : 0) This is what Xlib does. While less intuitive, it is actually more correct, if you follow this deduction: - The key group's type->mask defines which modifiers the key even cares about. The others are completely irrelevant (and in fact they are masked out from all sided in the level calculation). Example: NumLock for an alphabetic key. - The type->mask, the mods which are not masked out, are *all* relevant (and in fact in the level calculation they must match *exactly* to the state). These mods affect which level is chosen for the key, whether they are active or not. - Because the type->mask mods are all relevant, they must be considered as consumed by the calculation *even if they are not active*. Therefore we use type->mask instead of entry->mask. The second change is what happens when no entry is found: return 0 or just take preserve to be 0? Let's consider an example, the basic type type "ALPHABETIC" { modifiers = Shift+Lock; map[Shift] = Level2; map[Lock] = Level2; level_name[Level1] = "Base"; level_name[Level2] = "Caps"; }; Suppose Shift+Lock is active - it doesn't match any entry, thus it gets to level 0. The first interpretation would take them both to be unconsumed, the second (new one) would take them both to be consumed. This seems much better: Caps is active, and Shift disables it, they both do something. This change also fixes a pretty lousy bug (since 0.3.2), where Shift appears to apparently *not* disable Caps. What actually happens is that Caps is not consumed (see above) but active, thus the implicit capitalization in get_one_sym() kicks in and capitalizes it anyway. Reported-by: Davinder Pal Singh Bhamra Signed-off-by: Ran Benita <ran234@gmail.com>
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145
diff --git a/src/state.c b/src/state.c
index d130b8e..0f9ea79 100644
--- a/src/state.c
+++ b/src/state.c
@@ -1276,18 +1276,24 @@ xkb_state_led_name_is_active(struct xkb_state *state, const char *name)
static xkb_mod_mask_t
key_get_consumed(struct xkb_state *state, const struct xkb_key *key)
{
+ const struct xkb_key_type *type;
const struct xkb_key_type_entry *entry;
+ xkb_mod_mask_t preserve;
xkb_layout_index_t group;
group = xkb_state_key_get_layout(state, key->keycode);
if (group == XKB_LAYOUT_INVALID)
return 0;
+ type = key->groups[group].type;
+
entry = get_entry_for_key_state(state, key, group);
- if (!entry)
- return 0;
+ if (entry)
+ preserve = entry->preserve.mask;
+ else
+ preserve = 0;
- return entry->mods.mask & ~entry->preserve.mask;
+ return type->mods.mask & ~preserve;
}
/**
diff --git a/test/common.c b/test/common.c
index 8b3f954..e6cd1c3 100644
--- a/test/common.c
+++ b/test/common.c
@@ -64,6 +64,7 @@ test_key_seq_va(struct xkb_keymap *keymap, va_list ap)
xkb_keysym_t keysym;
const xkb_keysym_t *syms;
+ xkb_keysym_t sym;
unsigned int nsyms, i;
char ksbuf[64];
@@ -77,6 +78,11 @@ test_key_seq_va(struct xkb_keymap *keymap, va_list ap)
op = va_arg(ap, int);
nsyms = xkb_state_key_get_syms(state, kc, &syms);
+ if (nsyms == 1) {
+ sym = xkb_state_key_get_one_sym(state, kc);
+ syms = &sym;
+ }
+
fprintf(stderr, "got %u syms for key 0x%x: [", nsyms, kc);
if (op == DOWN || op == BOTH)
diff --git a/test/state.c b/test/state.c
index 97c2bb6..2164d6b 100644
--- a/test/state.c
+++ b/test/state.c
@@ -309,17 +309,25 @@ test_repeat(struct xkb_keymap *keymap)
static void
test_consume(struct xkb_keymap *keymap)
{
- struct xkb_state *state = xkb_state_new(keymap);
- xkb_mod_index_t alt, shift;
+ struct xkb_state *state;
+ xkb_mod_index_t alt, shift, caps, ctrl, mod5;
xkb_mod_mask_t mask;
+ state = xkb_state_new(keymap);
assert(state);
alt = xkb_keymap_mod_get_index(keymap, XKB_MOD_NAME_ALT);
assert(alt != XKB_MOD_INVALID);
shift = xkb_keymap_mod_get_index(keymap, XKB_MOD_NAME_SHIFT);
assert(shift != XKB_MOD_INVALID);
+ caps = xkb_keymap_mod_get_index(keymap, XKB_MOD_NAME_CAPS);
+ assert(caps != XKB_MOD_INVALID);
+ ctrl = xkb_keymap_mod_get_index(keymap, XKB_MOD_NAME_CTRL);
+ assert(ctrl != XKB_MOD_INVALID);
+ mod5 = xkb_keymap_mod_get_index(keymap, "Mod5");
+ assert(mod5 != XKB_MOD_INVALID);
+ /* Test remove_consumed() */
xkb_state_update_key(state, KEY_LEFTALT + EVDEV_OFFSET, XKB_KEY_DOWN);
xkb_state_update_key(state, KEY_LEFTSHIFT + EVDEV_OFFSET, XKB_KEY_DOWN);
xkb_state_update_key(state, KEY_EQUAL + EVDEV_OFFSET, XKB_KEY_DOWN);
@@ -333,9 +341,56 @@ test_consume(struct xkb_keymap *keymap)
mask);
assert(mask == (1U << alt));
+ /* Test get_consumed_mods() */
mask = xkb_state_key_get_consumed_mods(state, KEY_EQUAL + EVDEV_OFFSET);
assert(mask == (1U << shift));
+ mask = xkb_state_key_get_consumed_mods(state, KEY_ESC + EVDEV_OFFSET);
+ assert(mask == 0);
+
+ xkb_state_unref(state);
+
+ /* Test is_consumed() - simple ALPHABETIC type. */
+ state = xkb_state_new(keymap);
+ assert(state);
+
+ mask = xkb_state_key_get_consumed_mods(state, KEY_A + EVDEV_OFFSET);
+ assert(mask == ((1U << shift) | (1U << caps)));
+
+ assert(xkb_state_mod_index_is_consumed(state, KEY_A + EVDEV_OFFSET, caps) > 0);
+ assert(xkb_state_mod_index_is_consumed(state, KEY_A + EVDEV_OFFSET, shift) > 0);
+ xkb_state_update_key(state, KEY_CAPSLOCK + EVDEV_OFFSET, XKB_KEY_DOWN);
+ xkb_state_update_key(state, KEY_CAPSLOCK + EVDEV_OFFSET, XKB_KEY_UP);
+ assert(xkb_state_mod_index_is_consumed(state, KEY_A + EVDEV_OFFSET, caps) > 0);
+ assert(xkb_state_mod_index_is_consumed(state, KEY_A + EVDEV_OFFSET, shift) > 0);
+ xkb_state_update_key(state, KEY_LEFTSHIFT + EVDEV_OFFSET, XKB_KEY_DOWN);
+ assert(xkb_state_mod_index_is_consumed(state, KEY_A + EVDEV_OFFSET, caps) > 0);
+ assert(xkb_state_mod_index_is_consumed(state, KEY_A + EVDEV_OFFSET, shift) > 0);
+ xkb_state_update_key(state, KEY_LEFTSHIFT + EVDEV_OFFSET, XKB_KEY_UP);
+ xkb_state_update_key(state, KEY_CAPSLOCK + EVDEV_OFFSET, XKB_KEY_DOWN);
+ xkb_state_update_key(state, KEY_CAPSLOCK + EVDEV_OFFSET, XKB_KEY_UP);
+ assert(xkb_state_mod_index_is_consumed(state, KEY_A + EVDEV_OFFSET, caps) > 0);
+ assert(xkb_state_mod_index_is_consumed(state, KEY_A + EVDEV_OFFSET, shift) > 0);
+
+ xkb_state_unref(state);
+
+ /* More complicated - CTRL+ALT */
+ state = xkb_state_new(keymap);
+
+ mask = xkb_state_key_get_consumed_mods(state, KEY_F1 + EVDEV_OFFSET);
+ assert(mask == ((1U << shift) | (1U << alt) | (1U << ctrl) | (1U << mod5)));
+
+ /* Shift is preserved. */
+ xkb_state_update_key(state, KEY_LEFTSHIFT + EVDEV_OFFSET, XKB_KEY_DOWN);
+ mask = xkb_state_key_get_consumed_mods(state, KEY_F1 + EVDEV_OFFSET);
+ assert(mask == ((1U << alt) | (1U << ctrl) | (1U << mod5)));
+ xkb_state_update_key(state, KEY_LEFTSHIFT + EVDEV_OFFSET, XKB_KEY_UP);
+
+ mask = xkb_state_key_get_consumed_mods(state, KEY_F1 + EVDEV_OFFSET);
+ assert(mask == ((1U << shift) | (1U << alt) | (1U << ctrl) | (1U << mod5)));
+
+ assert(state);
+
xkb_state_unref(state);
}