From: Michael Stapelberg <michael@stapelberg.de>
Date: Wed, 24 May 2017 18:42:27 +0000 (+0200)
Subject: compare modifiers for equality, not subset (+test)
X-Git-Tag: 4.14~27^2
X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;ds=sidebyside;h=a4f6387911b4b79ca864052012c5cbace584eb2e;p=i3%2Fi3

compare modifiers for equality, not subset (+test)

Subset comparison was introduced with the rather large commit
bf3cd41b5ddf1e757515ab5fbf811be56e5f69cc, but I now think we should use
equality.

In other words, the following key binding:

    bindsym Mod4+x nop Mod4+x

previously would have been triggered when pressing Mod3+Mod4+x.

Strictly speaking, this is a change of behavior, but it breaks none of our
tests, and using equality instead of subset comparison enables more use-cases.

fixes #2002
---

diff --git a/src/bindings.c b/src/bindings.c
index 7cba556e..14eaaf5e 100644
--- a/src/bindings.c
+++ b/src/bindings.c
@@ -191,17 +191,6 @@ void regrab_all_buttons(xcb_connection_t *conn) {
     xcb_ungrab_server(conn);
 }
 
-static bool modifiers_match(const uint32_t modifiers_mask, const uint32_t modifiers_state) {
-    /* modifiers_mask is a special case: a value of 0 does not mean “match
-     * all”, but rather “match exactly when no modifiers are present”. */
-    if (modifiers_mask == 0) {
-        /* Verify no modifiers are pressed. A bitwise AND would lead to
-         * false positives, see issue #2002. */
-        return (modifiers_state == 0);
-    }
-    return ((modifiers_state & modifiers_mask) == modifiers_mask);
-}
-
 /*
  * Returns a pointer to the Binding with the specified modifiers and
  * keycode or NULL if no such binding exists.
@@ -244,7 +233,7 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas
             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);
+                const bool mods_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) {
@@ -265,7 +254,7 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas
             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);
+                const bool mods_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 (mods_match || (bind->release == B_UPON_KEYRELEASE_IGNORE_MODS && is_release)) {
diff --git a/testcases/t/264-keypress-numlock.t b/testcases/t/264-keypress-numlock.t
index 90a403af..2fdafb83 100644
--- a/testcases/t/264-keypress-numlock.t
+++ b/testcases/t/264-keypress-numlock.t
@@ -214,6 +214,8 @@ $config = <<EOT;
 # i3 config file (v4)
 font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
 
+bindsym Mod4+Return nop Return
+
 # Binding which should work with numlock and without, see issue #2559.
 bindcode --release 133 nop Super_L
 EOT
@@ -244,8 +246,34 @@ is(listen_for_binding(
    'Super_L',
    'triggered the "Super_L" keybinding with Num_Lock');
 
+is(listen_for_binding(
+    sub {
+        xtest_key_press(133); # Super_L
+        xtest_key_press(36); # Return
+        xtest_key_release(36); # Return
+        xtest_key_release(133); # Super_L
+    },
+    ),
+   'Return',
+   'triggered the "Return" 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(133); # Super_L
+        xtest_key_press(36); # Return
+        xtest_key_release(36); # Return
+        xtest_key_release(133); # Super_L
+        xtest_key_press(77); # disable Num_Lock
+        xtest_key_release(77); # disable Num_Lock
+    },
+    ),
+   'Return',
+   'triggered the "Return" keybinding with Num_Lock');
+
 sync_with_i3;
-is(scalar @i3test::XTEST::binding_events, 14, 'Received exactly 14 binding events');
+is(scalar @i3test::XTEST::binding_events, 16, 'Received exactly 16 binding events');
 
 exit_gracefully($pid);
 
@@ -312,7 +340,7 @@ is(listen_for_binding(
 # TODO: This test does not verify that i3 does _NOT_ grab keycode 87 with Mod2.
 
 sync_with_i3;
-is(scalar @i3test::XTEST::binding_events, 16, 'Received exactly 16 binding events');
+is(scalar @i3test::XTEST::binding_events, 18, 'Received exactly 18 binding events');
 
 exit_gracefully($pid);