Merge pull request #3179 from orestisf1993/issue-2733

Correctly handle bindings for the same key with and without --release

src/bindings.c

@@ -198,6 +198,7 @@ void regrab_all_buttons(xcb_connection_t *conn) {
  */
 static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_release, uint16_t input_code, input_type_t input_type) {
     Binding *bind;
+    Binding *result = NULL;
 
     if (!is_release) {
         /* On a press event, we first reset all B_UPON_KEYRELEASE_IGNORE_MODS
@@ -227,30 +228,27 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas
         /* For keyboard bindings where a symbol was specified by the user, we
          * need to look in the array of translated keycodes for the event’s
          * keycode */
+        bool found_keycode = false;
         if (input_type == B_KEYBOARD && bind->symbol != NULL) {
             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_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) {
+                if (binding_keycode->keycode == input_keycode &&
+                    (mods_match || (bind->release == B_UPON_KEYRELEASE_IGNORE_MODS && is_release))) {
                     found_keycode = true;
                     break;
                 }
             }
-            if (!found_keycode) {
-                continue;
-            }
         } else {
             /* This case is easier: The user specified a keycode */
             if (bind->keycode != input_code) {
                 continue;
             }
 
-            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);
@@ -262,9 +260,9 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas
                     break;
                 }
             }
-            if (!found_keycode) {
+        }
-                continue;
+        if (!found_keycode) {
-            }
+            continue;
         }
 
         /* If this binding is a release binding, it matches the key which the
@@ -274,23 +272,26 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas
         if (bind->release == B_UPON_KEYRELEASE && !is_release) {
             bind->release = B_UPON_KEYRELEASE_IGNORE_MODS;
             DLOG("marked bind %p as B_UPON_KEYRELEASE_IGNORE_MODS\n", bind);
-            /* The correct binding has been found, so abort the search, but
+            if (result) {
-             * also don’t return this binding, since it should not be executed
+                break;
-             * yet (only when the keys are released). */
+            }
-            bind = TAILQ_END(bindings);
-            break;
-        }
-
-        /* 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)) {
             continue;
         }
 
-        break;
+        /* Check if the binding is for a press or a release event */
+        if ((bind->release == B_UPON_KEYPRESS && is_release)) {
+            continue;
+        }
+
+        if (is_release) {
+            return bind;
+        } else if (!result) {
+            /* Continue looping to mark needed B_UPON_KEYRELEASE_IGNORE_MODS. */
+            result = bind;
+        }
     }
 
-    return (bind == TAILQ_END(bindings) ? NULL : bind);
+    return result;
 }
 
 /*
@@ -628,6 +629,14 @@ void switch_mode(const char *new_mode) {
         translate_keysyms();
         grab_all_keys(conn);
 
+        /* Reset all B_UPON_KEYRELEASE_IGNORE_MODS bindings to avoid possibly
+         * activating one of them. */
+        Binding *bind;
+        TAILQ_FOREACH(bind, bindings, bindings) {
+            if (bind->release == B_UPON_KEYRELEASE_IGNORE_MODS)
+                bind->release = B_UPON_KEYRELEASE;
+        }
+
         char *event_msg;
         sasprintf(&event_msg, "{\"change\":\"%s\", \"pango_markup\":%s}",
                   mode->name, (mode->pango_markup ? "true" : "false"));

testcases/t/258-keypress-release.t

@@ -29,6 +29,20 @@ bindsym --release Control+Print nop Control+Print
 # see issue #2442
 bindsym Mod1+b nop Mod1+b
 bindsym --release Mod1+Shift+b nop Mod1+Shift+b release
+
+bindsym --release Shift+x nop Shift+x
+
+# see issue #2733
+# 133 == Mod4
+bindcode 133 nop 133
+bindcode --release 133 nop 133 release
+
+mode "a_mode" {
+    # 27 == r
+    bindcode 27 --release mode "default"
+}
+bindsym Mod1+r mode "a_mode"
+bindcode 27 nop do not receive
 EOT
 use i3test::XTEST;
 use ExtUtils::PkgConfig;
@@ -85,6 +99,72 @@ is(listen_for_binding(
     'Mod1+Shift+b release',
     'triggered the "Mod1+Shift+b" release keybinding');
 
+is(listen_for_binding(
+        sub {
+            xtest_key_press(50); # Shift
+            xtest_key_press(53); # x
+            xtest_key_release(53); # x
+            xtest_key_release(50); # Shift
+            xtest_sync_with_i3;
+        },
+        ),
+       'Shift+x',
+       'triggered the "Shift+x" keybinding by releasing x first');
+
+is(listen_for_binding(
+        sub {
+            xtest_key_press(50); # Shift
+            xtest_key_press(53); # x
+            xtest_key_release(50); # Shift
+            xtest_key_release(53);  # x
+            xtest_sync_with_i3;
+        },
+        ),
+       'Shift+x',
+       'triggered the "Shift+x" keybinding by releasing Shift first');
+
+is(listen_for_binding(
+    sub {
+        xtest_key_press(133);
+        xtest_sync_with_i3;
+    },
+    ),
+    '133',
+    'triggered the 133 keycode press binding');
+
+is(listen_for_binding(
+    sub {
+        xtest_key_release(133);
+        xtest_sync_with_i3;
+    },
+    ),
+    '133 release',
+    'triggered the 133 keycode release binding');
+
+for my $i (1 .. 2) {
+    is(listen_for_binding(
+        sub {
+            xtest_key_press(64); # Alt_l
+            xtest_key_press(27); # r
+            xtest_key_release(27); # r
+            xtest_key_release(64); # Alt_l
+            xtest_sync_with_i3;
+        },
+        ),
+        'mode "a_mode"',
+        "switched to mode \"a_mode\" $i/2");
+
+    is(listen_for_binding(
+        sub {
+            xtest_key_press(27); # r
+            xtest_key_release(27); # r
+            xtest_sync_with_i3;
+        },
+        ),
+        'mode "default"',
+        "switched back to default $i/2");
+}
+
 }
 
 done_testing;