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

diff --git a/src/bindings.c b/src/bindings.c
index 22870d3cc4f1d1d44c50a1f7d5dd6a4455c52493..e0b70b6c058723796bfcb3d60e8f374cdd31a120 100644 (file)
--- 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 (file)
index 0000000..84db0e3
--- /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 = <<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;