From d6a715b9769a4be0eeb9cbc2af221c77f2d2a907 Mon Sep 17 00:00:00 2001 From: Jules Bertholet Date: Wed, 8 Jan 2025 11:09:06 -0500 Subject: [PATCH] state: Fix mods not being independently cleared The modifiers filters should ensure minimal interaction between them, but currently the Latch mod filters are overzealous and mess with the mods from other filters set to be cleared, resulting in some modifiers permanently set. Fixed by clearing mods properly with `OR` rather than direct setting of `state::clear_mods`. While we are at it, `state::set_mods` should be `OR`ed as well. This should not have any impact for now, but this is more future-proof. Fixes #583 Co-authored-by: Jules Bertholet Co-authored-by: Pierre Le Marre --- changes/api/583.bugfix.md | 1 + src/state.c | 12 ++-- test/data/symbols/simultaneous-mods-latches | 16 +++++ test/keyseq.c | 79 ++++++++++++++++++++- 4 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 changes/api/583.bugfix.md create mode 100644 test/data/symbols/simultaneous-mods-latches diff --git a/changes/api/583.bugfix.md b/changes/api/583.bugfix.md new file mode 100644 index 000000000..8d2bd699b --- /dev/null +++ b/changes/api/583.bugfix.md @@ -0,0 +1 @@ +Fixed modifiers not properly unset when multiple latches are used simultaneously. diff --git a/src/state.c b/src/state.c index f48c5858b..6a5e5f30c 100644 --- a/src/state.c +++ b/src/state.c @@ -454,7 +454,7 @@ xkb_filter_group_latch_func(struct xkb_state *state, static void xkb_filter_mod_set_new(struct xkb_state *state, struct xkb_filter *filter) { - state->set_mods = filter->action.mods.mods.mask; + state->set_mods |= filter->action.mods.mods.mask; } static bool @@ -476,7 +476,7 @@ xkb_filter_mod_set_func(struct xkb_state *state, return XKB_FILTER_CONSUME; } - state->clear_mods = filter->action.mods.mods.mask; + state->clear_mods |= filter->action.mods.mods.mask; if (filter->action.mods.flags & ACTION_LOCK_CLEAR) state->components.locked_mods &= ~filter->action.mods.mods.mask; @@ -522,7 +522,7 @@ static void xkb_filter_mod_latch_new(struct xkb_state *state, struct xkb_filter *filter) { filter->priv = LATCH_KEY_DOWN; - state->set_mods = filter->action.mods.mods.mask; + state->set_mods |= filter->action.mods.mods.mask; } static bool @@ -553,7 +553,7 @@ xkb_filter_mod_latch_func(struct xkb_state *state, else { filter->action.type = ACTION_TYPE_MOD_SET; filter->func = xkb_filter_mod_set_func; - state->set_mods = filter->action.mods.mods.mask; + state->set_mods |= filter->action.mods.mods.mask; } filter->key = key; state->components.latched_mods &= ~filter->action.mods.mods.mask; @@ -585,13 +585,13 @@ xkb_filter_mod_latch_func(struct xkb_state *state, state->components.latched_mods &= ~filter->action.mods.mods.mask; else - state->clear_mods = filter->action.mods.mods.mask; + state->clear_mods |= filter->action.mods.mods.mask; state->components.locked_mods &= ~filter->action.mods.mods.mask; filter->func = NULL; } else { latch = LATCH_PENDING; - state->clear_mods = filter->action.mods.mods.mask; + state->clear_mods |= filter->action.mods.mods.mask; state->components.latched_mods |= filter->action.mods.mods.mask; /* XXX beep beep! */ } diff --git a/test/data/symbols/simultaneous-mods-latches b/test/data/symbols/simultaneous-mods-latches new file mode 100644 index 000000000..181020c98 --- /dev/null +++ b/test/data/symbols/simultaneous-mods-latches @@ -0,0 +1,16 @@ +default partial alphanumeric_keys +xkb_symbols "base" { + name[Group1] = "Test issue #583"; + + key { [ z, Z, y, Y, ezh, EZH, ydiaeresis, Ydiaeresis ], type[Group1] = "EIGHT_LEVEL" }; + key { [ x, X ], type[Group1] = "PC_CONTROL_LEVEL2" }; + + key { [ ISO_Level3_Latch, ISO_Level5_Latch ], type[Group1] = "PC_CONTROL_LEVEL2" }; + key { [ Control_R ], [LatchMods(modifiers=Control)] }; + key { [ ISO_Level3_Latch ], type[Group1] = "ONE_LEVEL" }; + key { [ ISO_Level5_Latch ], type[Group1] = "ONE_LEVEL" }; + + modifier_map None { }; + modifier_map Mod3 { }; + modifier_map Mod5 { }; +}; diff --git a/test/keyseq.c b/test/keyseq.c index ebacfa3da..c2f4e5bc1 100644 --- a/test/keyseq.c +++ b/test/keyseq.c @@ -374,6 +374,82 @@ test_explicit_actions(struct xkb_context *ctx) } } +static void +test_simultaneous_modifier_clear(struct xkb_context *context) +{ + struct xkb_keymap *keymap; + + keymap = test_compile_rules(context, "evdev", "pc104", + "simultaneous-mods-latches", "", ""); + assert(keymap); + + /* + * Github issue #583: simultaneous latches of *different* modifiers should + * not affect each other when clearing their mods. + */ + + /* Original key sequence reported in the issue */ + assert(test_key_seq(keymap, + KEY_LEFTCTRL, DOWN, XKB_KEY_Control_L , NEXT, /* Set Control */ + KEY_RIGHTALT, BOTH, XKB_KEY_ISO_Level5_Latch, NEXT, /* Latch Level5 */ + KEY_LEFTCTRL, UP , XKB_KEY_Control_L , NEXT, /* Unset Control */ + KEY_RIGHTALT, BOTH, XKB_KEY_ISO_Level3_Latch, NEXT, /* Latch Level3 */ + KEY_Z , BOTH, XKB_KEY_ydiaeresis , NEXT, /* Unlatch Level3, unlatch Level5 */ + KEY_Z , BOTH, XKB_KEY_z , NEXT, + KEY_Z , BOTH, XKB_KEY_z , FINISH + )); + + /* Alternative key sequence with only mod latches */ + assert(test_key_seq(keymap, + KEY_RIGHTCTRL, BOTH, XKB_KEY_Control_R , NEXT, /* Latch Control */ + KEY_RIGHTALT, BOTH, XKB_KEY_ISO_Level5_Latch, NEXT, /* Latch Level5 */ + KEY_LEFTMETA, BOTH, XKB_KEY_ISO_Level3_Latch, NEXT, /* Latch Level3 */ + KEY_Z , BOTH, XKB_KEY_ydiaeresis , NEXT, /* Unlatch Control, Level3 and Level5 */ + KEY_Z , BOTH, XKB_KEY_z , NEXT, + KEY_Z , BOTH, XKB_KEY_z , NEXT, + KEY_X , BOTH, XKB_KEY_x , FINISH + )); + + /* Alternative simplier key sequence */ + assert(test_key_seq(keymap, + KEY_LEFTMETA, BOTH, XKB_KEY_ISO_Level3_Latch, NEXT, /* Latch Level3 */ + KEY_RIGHTMETA, BOTH, XKB_KEY_ISO_Level5_Latch, NEXT, /* Latch Level5 */ + KEY_Z , BOTH, XKB_KEY_ydiaeresis , NEXT, /* Unlatch Level3, unlatch Level5 */ + KEY_Z , BOTH, XKB_KEY_z , NEXT, + KEY_Z , BOTH, XKB_KEY_z , FINISH + )); + + /* + * Test same modifier latch but on a different key + */ + + /* Level 3 */ + assert(test_key_seq(keymap, + KEY_LEFTMETA, BOTH, XKB_KEY_ISO_Level3_Latch, NEXT, /* Latch Level3 */ + KEY_RIGHTALT, BOTH, XKB_KEY_ISO_Level3_Latch, NEXT, /* Lock Level3 via latch */ + KEY_Z , BOTH, XKB_KEY_y , NEXT, /* Locked Level3 */ + KEY_Z , BOTH, XKB_KEY_y , NEXT, + KEY_RIGHTALT, BOTH, XKB_KEY_ISO_Level3_Latch, NEXT, /* Unlock Level3 via latch */ + KEY_Z , BOTH, XKB_KEY_z , NEXT, + KEY_Z , BOTH, XKB_KEY_z , FINISH + )); + + /* Level 5, via Control latch */ + assert(test_key_seq(keymap, + KEY_RIGHTCTRL, BOTH, XKB_KEY_Control_R , NEXT, /* Latch Control */ + KEY_RIGHTALT, BOTH, XKB_KEY_ISO_Level5_Latch, NEXT, /* Lock Level5 via latch */ + KEY_RIGHTMETA, BOTH, XKB_KEY_ISO_Level5_Latch, NEXT, /* Latch Level5 */ + KEY_Z , BOTH, XKB_KEY_ezh , NEXT, /* Locked Level5 */ + KEY_Z , BOTH, XKB_KEY_ezh , NEXT, + KEY_RIGHTMETA, BOTH, XKB_KEY_ISO_Level5_Latch, NEXT, /* Unlock Level5 via latch */ + KEY_Z , BOTH, XKB_KEY_z , NEXT, + KEY_Z , BOTH, XKB_KEY_z , NEXT, + KEY_X , BOTH, XKB_KEY_x , FINISH + )); + + xkb_keymap_unref(keymap); +} + int main(void) { @@ -384,6 +460,7 @@ main(void) assert(ctx); + test_simultaneous_modifier_clear(ctx); test_group_latch(ctx); test_explicit_actions(ctx); @@ -839,5 +916,5 @@ main(void) xkb_keymap_unref(keymap); xkb_context_unref(ctx); - return 0; + return EXIT_SUCCESS; }