From d77d40173aae250d2e5ba86a8bce7a6e857cbc67 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Tue, 2 Aug 2016 20:23:53 +0200 Subject: [PATCH] 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 3bd5e6e5c81b448f9f0ac84454671a871fbfea66 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 --- src/bindings.c | 50 +++++++++-- testcases/t/264-keypress-numlock.t | 138 +++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 8 deletions(-) create mode 100644 testcases/t/264-keypress-numlock.t diff --git a/src/bindings.c b/src/bindings.c index 22870d3c..e0b70b6c 100644 --- a/src/bindings.c +++ b/src/bindings.c @@ -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 don’t + * 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); diff --git a/testcases/t/264-keypress-numlock.t b/testcases/t/264-keypress-numlock.t new file mode 100644 index 00000000..84db0e3c --- /dev/null +++ b/testcases/t/264-keypress-numlock.t @@ -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 = <