Skip to content

Commit

Permalink
state: Fix mods not being independently cleared
Browse files Browse the repository at this point in the history 
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 xkbcommon#583

Co-authored-by: Jules Bertholet <[email protected]>
Co-authored-by: Pierre Le Marre <[email protected]>
  • Loading branch information
@Jules-Bertholet @wismill
Jules-Bertholet and wismill committed Jan 16, 2025
1 parent dfa286b commit d6a715b
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 7 deletions.
1 change: 1 addition & 0 deletions changes/api/583.bugfix.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed modifiers not properly unset when multiple latches are used simultaneously.
12 changes: 6 additions & 6 deletions src/state.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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! */
}
Expand Down
16 changes: 16 additions & 0 deletions test/data/symbols/simultaneous-mods-latches
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
default partial alphanumeric_keys
xkb_symbols "base" {
name[Group1] = "Test issue #583";

key <AB01> { [ z, Z, y, Y, ezh, EZH, ydiaeresis, Ydiaeresis ], type[Group1] = "EIGHT_LEVEL" };
key <AB02> { [ x, X ], type[Group1] = "PC_CONTROL_LEVEL2" };

key <RALT> { [ ISO_Level3_Latch, ISO_Level5_Latch ], type[Group1] = "PC_CONTROL_LEVEL2" };
key <RCTL> { [ Control_R ], [LatchMods(modifiers=Control)] };
key <LWIN> { [ ISO_Level3_Latch ], type[Group1] = "ONE_LEVEL" };
key <RWIN> { [ ISO_Level5_Latch ], type[Group1] = "ONE_LEVEL" };

modifier_map None { <RALT> };
modifier_map Mod3 { <RWIN> };
modifier_map Mod5 { <LWIN> };
};
79 changes: 78 additions & 1 deletion test/keyseq.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -384,6 +460,7 @@ main(void)

assert(ctx);

test_simultaneous_modifier_clear(ctx);
test_group_latch(ctx);
test_explicit_actions(ctx);

Expand Down Expand Up @@ -839,5 +916,5 @@ main(void)

xkb_keymap_unref(keymap);
xkb_context_unref(ctx);
return 0;
return EXIT_SUCCESS;
}

0 comments on commit d6a715b

Please sign in to comment.