Only add NumLock fallback where necessary.

Previously, we always discarded the NumLock bit when looking up key
bindings for key press events, and we always grabbed every keycode with
and without the NumLock modifier.

With this commit, the NumLock bit is no longer discarded: since the
previous commit 3bd5e6e5c8 we can
correctly look up key bindings with/without the NumLock bit, as both
variants are stored in |keycodes_head|.

Further, before adding the NumLock fallback (resulting in grabbing the
keycode with the NumLock modifier), we now check whether the key has the
same meaning when NumLock is enabled. This correctly distinguishes the
KP_End vs. KP_1 case, i.e. one can now use the following key bindings:

    # No longer accidentally triggered when pressing KP_1.
    bindsym KP_End nop KP_End

    # Properly distinguished now:
    bindsym KP_End nop KP_End
    bindsym Mod2+KP_1 nop KP_1

fixes #2346
This commit is contained in:
Michael Stapelberg 2016-08-02 20:23:53 +02:00
parent 3bd5e6e5c8
commit d77d40173a
2 changed files with 180 additions and 8 deletions

View File

@ -295,9 +295,9 @@ Binding *get_binding_from_xcb_event(xcb_generic_event_t *event) {
const uint16_t event_state = ((xcb_key_press_event_t *)event)->state;
const uint16_t event_detail = ((xcb_key_press_event_t *)event)->detail;
/* Remove the numlock bit */
i3_event_state_mask_t state_filtered = event_state & ~(xcb_numlock_mask | XCB_MOD_MASK_LOCK);
DLOG("(removed numlock, state = 0x%x)\n", state_filtered);
/* Remove the CapsLock bit */
i3_event_state_mask_t state_filtered = event_state & ~XCB_MOD_MASK_LOCK;
DLOG("(removed capslock, state = 0x%x)\n", state_filtered);
/* Transform the keyboard_group from bit 13 and bit 14 into an
* i3_xkb_group_mask_t, so that get_binding() can just bitwise AND the
* configured bindings against |state_filtered|.
@ -338,6 +338,9 @@ struct resolve {
/* Like |xkb_state|, just without the shift modifier, if shift was specified. */
struct xkb_state *xkb_state_no_shift;
/* Like |xkb_state|, but with NumLock. */
struct xkb_state *xkb_state_numlock;
};
/*
@ -373,14 +376,28 @@ static void add_keycode_if_matches(struct xkb_keymap *keymap, xkb_keycode_t key,
ADD_TRANSLATED_KEY(bind->event_state_mask);
/* Also bind the key with active NumLock */
ADD_TRANSLATED_KEY(bind->event_state_mask | xcb_numlock_mask);
/* Also bind the key with active CapsLock */
ADD_TRANSLATED_KEY(bind->event_state_mask | XCB_MOD_MASK_LOCK);
/* Also bind the key with active NumLock+CapsLock */
ADD_TRANSLATED_KEY(bind->event_state_mask | xcb_numlock_mask | XCB_MOD_MASK_LOCK);
/* If this binding is not explicitly for NumLock, check whether we need to
* add a fallback. */
if ((bind->event_state_mask & xcb_numlock_mask) != xcb_numlock_mask) {
/* Check whether the keycode results in the same keysym when NumLock is
* active. If so, grab the key with NumLock as well, so that users dont
* need to duplicate every key binding with an additional Mod2 specified.
*/
xkb_keysym_t sym_numlock = xkb_state_key_get_one_sym(resolving->xkb_state_numlock, key);
if (sym_numlock == resolving->keysym) {
/* Also bind the key with active NumLock */
ADD_TRANSLATED_KEY(bind->event_state_mask | xcb_numlock_mask);
/* Also bind the key with active NumLock+CapsLock */
ADD_TRANSLATED_KEY(bind->event_state_mask | xcb_numlock_mask | XCB_MOD_MASK_LOCK);
} else {
DLOG("Skipping automatic numlock fallback, key %d resolves to 0x%x with unmlock\n",
key, sym_numlock);
}
}
#undef ADD_TRANSLATED_KEY
}
@ -402,6 +419,12 @@ void translate_keysyms(void) {
return;
}
struct xkb_state *dummy_state_numlock = xkb_state_new(xkb_keymap);
if (dummy_state_numlock == NULL) {
ELOG("Could not create XKB state, cannot translate keysyms.\n");
return;
}
bool has_errors = false;
Binding *bind;
TAILQ_FOREACH(bind, bindings, bindings) {
@ -458,11 +481,21 @@ void translate_keysyms(void) {
0 /* xkb_layout_index_t latched_group, */,
group /* xkb_layout_index_t locked_group, */);
(void)xkb_state_update_mask(
dummy_state_numlock,
(bind->event_state_mask & 0x1FFF) | xcb_numlock_mask /* xkb_mod_mask_t base_mods, */,
0 /* xkb_mod_mask_t latched_mods, */,
0 /* xkb_mod_mask_t locked_mods, */,
0 /* xkb_layout_index_t base_group, */,
0 /* xkb_layout_index_t latched_group, */,
group /* xkb_layout_index_t locked_group, */);
struct resolve resolving = {
.bind = bind,
.keysym = keysym,
.xkb_state = dummy_state,
.xkb_state_no_shift = dummy_state_no_shift,
.xkb_state_numlock = dummy_state_numlock,
};
while (!TAILQ_EMPTY(&(bind->keycodes_head))) {
struct Binding_Keycode *first = TAILQ_FIRST(&(bind->keycodes_head));
@ -502,6 +535,7 @@ void translate_keysyms(void) {
xkb_state_unref(dummy_state);
xkb_state_unref(dummy_state_no_shift);
xkb_state_unref(dummy_state_numlock);
if (has_errors) {
start_config_error_nagbar(current_configpath, true);

View File

@ -0,0 +1,138 @@
#!perl
# vim:ts=4:sw=4:expandtab
#
# Please read the following documents before working on tests:
# • http://build.i3wm.org/docs/testsuite.html
# (or docs/testsuite)
#
# • http://build.i3wm.org/docs/lib-i3test.html
# (alternatively: perldoc ./testcases/lib/i3test.pm)
#
# • http://build.i3wm.org/docs/ipc.html
# (or docs/ipc)
#
# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf
# (unless you are already familiar with Perl)
#
# Verifies that one can bind on numpad keys in different numlock states.
# Ticket: #2346
# Bug still in: 4.12-78-g85bb324
use i3test i3_autostart => 0;
use i3test::XTEST;
use ExtUtils::PkgConfig;
SKIP: {
skip "libxcb-xkb too old (need >= 1.11)", 1 unless
ExtUtils::PkgConfig->atleast_version('xcb-xkb', '1.11');
my $config = <<EOT;
# i3 config file (v4)
font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
# Same key, different numlock states.
bindsym Mod2+KP_1 nop KP_1
bindsym KP_End nop KP_End
# Binding which should work with numlock and without.
bindsym Mod4+a nop a
EOT
my $pid = launch_with_config($config);
start_binding_capture;
is(listen_for_binding(
sub {
xtest_key_press(87); # KP_End
xtest_key_release(87); # KP_End
},
),
'KP_End',
'triggered the "KP_End" keybinding');
is(listen_for_binding(
sub {
xtest_key_press(77); # Num_Lock
xtest_key_press(87); # KP_1
xtest_key_release(87); # KP_1
xtest_key_release(77); # Num_Lock
},
),
'KP_1',
'triggered the "KP_1" keybinding');
is(listen_for_binding(
sub {
xtest_key_press(133); # Super_L
xtest_key_press(38); # a
xtest_key_release(38); # a
xtest_key_release(133); # Super_L
},
),
'a',
'triggered the "a" keybinding');
is(listen_for_binding(
sub {
xtest_key_press(77); # Num_Lock
xtest_key_press(133); # Super_L
xtest_key_press(38); # a
xtest_key_release(38); # a
xtest_key_release(133); # Super_L
xtest_key_release(77); # Num_Lock
},
),
'a',
'triggered the "a" keybinding');
sync_with_i3;
is(scalar @i3test::XTEST::binding_events, 4, 'Received exactly 4 binding events');
exit_gracefully($pid);
################################################################################
# Verify the binding is only triggered for KP_End, not KP_1
################################################################################
$config = <<EOT;
# i3 config file (v4)
font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
bindsym KP_End nop KP_End
EOT
$pid = launch_with_config($config);
start_binding_capture;
is(listen_for_binding(
sub {
xtest_key_press(87); # KP_End
xtest_key_release(87); # KP_End
},
),
'KP_End',
'triggered the "KP_End" keybinding');
is(listen_for_binding(
sub {
xtest_key_press(77); # Num_Lock
xtest_key_press(87); # KP_1
xtest_key_release(87); # KP_1
xtest_key_release(77); # Num_Lock
},
),
'timeout',
'Did not trigger the KP_End keybinding with KP_1');
# TODO: This test does not verify that i3 does _NOT_ grab keycode 87 with Mod2.
sync_with_i3;
is(scalar @i3test::XTEST::binding_events, 5, 'Received exactly 5 binding events');
exit_gracefully($pid);
}
done_testing;