Apply numlock fallback to bindcode where necessary (#2781)

This was broken with commit d77d40173aae250d2e5ba86a8bce7a6e857cbc67

fixes #2559

diff --git a/src/bindings.c b/src/bindings.c
index bf04c4320be42de4ea63b479129a46489acc6657..8d54c6ebe30fc5103fd4e841357527304a6b3707 100644 (file)
--- a/src/bindings.c
+++ b/src/bindings.c
@@ -224,8 +224,9 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas
     const uint32_t xkb_group_state = (state_filtered & 0xFFFF0000);
     const uint32_t modifiers_state = (state_filtered & 0x0000FFFF);
     TAILQ_FOREACH(bind, bindings, bindings) {
-        if (bind->input_type != input_type)
+        if (bind->input_type != input_type) {
             continue;
+        }
 
         const uint32_t xkb_group_mask = (bind->event_state_mask & 0xFFFF0000);
         const bool groups_match = ((xkb_group_state & xkb_group_mask) == xkb_group_mask);
@@ -251,23 +252,31 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas
                     break;
                 }
             }
-            if (!found_keycode)
+            if (!found_keycode) {
                 continue;
+            }
         } else {
-            const uint32_t modifiers_mask = (bind->event_state_mask & 0x0000FFFF);
-            const bool mods_match = modifiers_match(modifiers_mask, modifiers_state);
-            DLOG("binding mods_match = %s\n", (mods_match ? "yes" : "no"));
-            /* First compare the state_filtered (unless this is a
-             * B_UPON_KEYRELEASE_IGNORE_MODS binding and this is a KeyRelease
-             * event) */
-            if (!mods_match &&
-                (bind->release != B_UPON_KEYRELEASE_IGNORE_MODS ||
-                 !is_release))
+            /* This case is easier: The user specified a keycode */
+            if (bind->keycode != input_code) {
                 continue;
+            }
 
-            /* This case is easier: The user specified a keycode */
-            if (bind->keycode != input_code)
+            xcb_keycode_t input_keycode = (xcb_keycode_t)input_code;
+            bool found_keycode = false;
+            struct Binding_Keycode *binding_keycode;
+            TAILQ_FOREACH(binding_keycode, &(bind->keycodes_head), keycodes) {
+                const uint32_t modifiers_mask = (binding_keycode->modifiers & 0x0000FFFF);
+                const bool mods_match = modifiers_match(modifiers_mask, modifiers_state);
+                DLOG("binding_keycode->modifiers = %d, modifiers_mask = %d, modifiers_state = %d, mods_match = %s\n",
+                     binding_keycode->modifiers, modifiers_mask, modifiers_state, (mods_match ? "yes" : "no"));
+                if (binding_keycode->keycode == input_keycode && mods_match) {
+                    found_keycode = true;
+                    break;
+                }
+            }
+            if (!found_keycode) {
                 continue;
+            }
         }
 
         /* If this binding is a release binding, it matches the key which the
@@ -286,8 +295,9 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas
 
         /* Check if the binding is for a press or a release event */
         if ((bind->release == B_UPON_KEYPRESS && is_release) ||
-            (bind->release >= B_UPON_KEYRELEASE && !is_release))
+            (bind->release >= B_UPON_KEYRELEASE && !is_release)) {
             continue;
+        }
 
         break;
     }
@@ -460,6 +470,14 @@ void translate_keysyms(void) {
     bool has_errors = false;
     Binding *bind;
     TAILQ_FOREACH(bind, bindings, bindings) {
+#define ADD_TRANSLATED_KEY(code, mods)                                                     \
+    do {                                                                                   \
+        struct Binding_Keycode *binding_keycode = smalloc(sizeof(struct Binding_Keycode)); \
+        binding_keycode->modifiers = (mods);                                               \
+        binding_keycode->keycode = (code);                                                 \
+        TAILQ_INSERT_TAIL(&(bind->keycodes_head), binding_keycode, keycodes);              \
+    } while (0)
+
         if (bind->input_type == B_MOUSE) {
             long button;
             if (!parse_long(bind->symbol + (sizeof("button") - 1), &button, 10)) {
@@ -467,17 +485,7 @@ void translate_keysyms(void) {
             }
 
             bind->keycode = button;
-            continue;
-        }
-
-        if (bind->keycode > 0)
-            continue;
-
-        /* We need to translate the symbol to a keycode */
-        const xkb_keysym_t keysym = xkb_keysym_from_name(bind->symbol, XKB_KEYSYM_NO_FLAGS);
-        if (keysym == XKB_KEY_NoSymbol) {
-            ELOG("Could not translate string to key symbol: \"%s\"\n",
-                 bind->symbol);
+            ADD_TRANSLATED_KEY(button, bind->event_state_mask);
             continue;
         }
 
@@ -532,6 +540,52 @@ void translate_keysyms(void) {
             0 /* xkb_layout_index_t latched_group, */,
             group /* xkb_layout_index_t locked_group, */);
 
+        if (bind->keycode > 0) {
+            /* We need to specify modifiers for the keycode binding (numlock
+             * fallback). */
+            while (!TAILQ_EMPTY(&(bind->keycodes_head))) {
+                struct Binding_Keycode *first = TAILQ_FIRST(&(bind->keycodes_head));
+                TAILQ_REMOVE(&(bind->keycodes_head), first, keycodes);
+                FREE(first);
+            }
+
+            ADD_TRANSLATED_KEY(bind->keycode, bind->event_state_mask);
+
+            /* Also bind the key with active CapsLock */
+            ADD_TRANSLATED_KEY(bind->keycode, bind->event_state_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 = xkb_state_key_get_one_sym(dummy_state, bind->keycode);
+                xkb_keysym_t sym_numlock = xkb_state_key_get_one_sym(dummy_state_numlock, bind->keycode);
+                if (sym == sym_numlock) {
+                    /* Also bind the key with active NumLock */
+                    ADD_TRANSLATED_KEY(bind->keycode, bind->event_state_mask | xcb_numlock_mask);
+
+                    /* Also bind the key with active NumLock+CapsLock */
+                    ADD_TRANSLATED_KEY(bind->keycode, bind->event_state_mask | xcb_numlock_mask | XCB_MOD_MASK_LOCK);
+                } else {
+                    DLOG("Skipping automatic numlock fallback, key %d resolves to 0x%x with numlock\n",
+                         bind->keycode, sym_numlock);
+                }
+            }
+
+            continue;
+        }
+
+        /* We need to translate the symbol to a keycode */
+        const xkb_keysym_t keysym = xkb_keysym_from_name(bind->symbol, XKB_KEYSYM_NO_FLAGS);
+        if (keysym == XKB_KEY_NoSymbol) {
+            ELOG("Could not translate string to key symbol: \"%s\"\n",
+                 bind->symbol);
+            continue;
+        }
+
         struct resolve resolving = {
             .bind = bind,
             .keysym = keysym,
@@ -574,6 +628,8 @@ void translate_keysyms(void) {
         DLOG("state=0x%x, cfg=\"%s\", sym=0x%x → keycodes%s (%d)\n",
              bind->event_state_mask, bind->symbol, keysym, keycodes, num_keycodes);
         free(keycodes);
+
+#undef ADD_TRANSLATED_KEY
     }
 
     xkb_state_unref(dummy_state);

diff --git a/testcases/t/264-keypress-numlock.t b/testcases/t/264-keypress-numlock.t
index 45ec7e8897163e00a727f11fe21143ab6234a2d6..464fe7481a7fc97752db3368a38cd250f6c9a333 100644 (file)
--- a/testcases/t/264-keypress-numlock.t
+++ b/testcases/t/264-keypress-numlock.t
@@ -44,6 +44,9 @@ bindsym Shift+Escape nop Shift+Escape
 
 # Binding which should work with numlock and without, see issue #2418.
 bindsym Mod1+Shift+q nop Mod1+Shift+q
+
+# Binding which should work with numlock and without, see issue #2559.
+bindcode 39 nop s
 EOT
 
 my $pid = launch_with_config($config);
@@ -159,7 +162,6 @@ is(listen_for_binding(
    'Mod1+Shift+q',
    'triggered the "Mod1+Shift+q" keybinding');
 
-
 is(listen_for_binding(
     sub {
         xtest_key_press(77); # enable Num_Lock
@@ -177,8 +179,30 @@ is(listen_for_binding(
    'Mod1+Shift+q',
    'triggered the "Mod1+Shift+q" keybinding');
 
+is(listen_for_binding(
+    sub {
+        xtest_key_press(39); # s
+        xtest_key_release(39); # s
+    },
+    ),
+   's',
+   'triggered the "s" keybinding without Num_Lock');
+
+is(listen_for_binding(
+    sub {
+        xtest_key_press(77); # enable Num_Lock
+        xtest_key_release(77); # enable Num_Lock
+        xtest_key_press(39); # s
+        xtest_key_release(39); # s
+        xtest_key_press(77); # disable Num_Lock
+        xtest_key_release(77); # disable Num_Lock
+    },
+    ),
+   's',
+   'triggered the "s" keybinding with Num_Lock');
+
 sync_with_i3;
-is(scalar @i3test::XTEST::binding_events, 10, 'Received exactly 10 binding events');
+is(scalar @i3test::XTEST::binding_events, 12, 'Received exactly 12 binding events');
 
 exit_gracefully($pid);
 
@@ -191,6 +215,7 @@ $config = <<EOT;
 font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
 
 bindsym KP_End nop KP_End
+bindcode 88 nop KP_Down
 EOT
 
 $pid = launch_with_config($config);
@@ -206,6 +231,15 @@ is(listen_for_binding(
    'KP_End',
    'triggered the "KP_End" keybinding');
 
+is(listen_for_binding(
+    sub {
+        xtest_key_press(88); # KP_Down
+        xtest_key_release(88); # KP_Down
+    },
+    ),
+   'KP_Down',
+   'triggered the "KP_Down" keybinding');
+
 is(listen_for_binding(
     sub {
         xtest_key_press(77); # enable Num_Lock
@@ -219,10 +253,23 @@ is(listen_for_binding(
    'timeout',
    'Did not trigger the KP_End keybinding with KP_1');
 
+is(listen_for_binding(
+    sub {
+        xtest_key_press(77); # enable Num_Lock
+        xtest_key_release(77); # enable Num_Lock
+        xtest_key_press(88); # KP_2
+        xtest_key_release(88); # KP_2
+        xtest_key_press(77); # disable Num_Lock
+        xtest_key_release(77); # disable Num_Lock
+    },
+    ),
+   'timeout',
+   'Did not trigger the KP_Down keybinding with KP_2');
+
 # TODO: This test does not verify that i3 does _NOT_ grab keycode 87 with Mod2.
 
 sync_with_i3;
-is(scalar @i3test::XTEST::binding_events, 11, 'Received exactly 11 binding events');
+is(scalar @i3test::XTEST::binding_events, 14, 'Received exactly 14 binding events');
 
 exit_gracefully($pid);