From 9d22d2efcea7ed95991ce4417d0feea2e2529af5 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Fri, 16 Mar 2018 03:08:47 +0200 Subject: [PATCH 1/4] Reduce repetition in get_binding() --- src/bindings.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/bindings.c b/src/bindings.c index c145b9560..9edc04dcf 100644 --- a/src/bindings.c +++ b/src/bindings.c @@ -227,9 +227,9 @@ 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); @@ -241,16 +241,12 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas 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 +258,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 From 130b3ce3a9cf249e96719f8f90f2ac462112cca7 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Sat, 17 Mar 2018 23:28:23 +0200 Subject: [PATCH 2/4] Check for B_UPON_KEYRELEASE_IGNORE_MODS with bindsyms From 548d74015c50d7fae14bfb8bb1989acde5fc22ae: > 1. press $mod, press x, release x, release $mod > 2. press $mod, press x, release $mod, release x case (2.) didn't work, now it should be fixed. --- src/bindings.c | 3 ++- testcases/t/258-keypress-release.t | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/bindings.c b/src/bindings.c index 9edc04dcf..823730ff6 100644 --- a/src/bindings.c +++ b/src/bindings.c @@ -236,7 +236,8 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas 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; } diff --git a/testcases/t/258-keypress-release.t b/testcases/t/258-keypress-release.t index 8bca0d869..b92f723fd 100644 --- a/testcases/t/258-keypress-release.t +++ b/testcases/t/258-keypress-release.t @@ -29,6 +29,8 @@ 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 EOT use i3test::XTEST; use ExtUtils::PkgConfig; @@ -85,6 +87,29 @@ 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'); } done_testing; From ff579ef22f7ba2fa4762564f84bfa062e79cfaa2 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Fri, 16 Mar 2018 03:38:13 +0200 Subject: [PATCH 3/4] Correctly handle bindings for the same mod key with and without --release Before this commit, get_binding() exited on the first match without marking the rest --release bindings with B_UPON_KEYRELEASE_IGNORE_MODS. Similarly, once it found a --release binding during a KeyPress event it would stop searching for a matching key press binding. Example config, placing the --release line first will trigger the second problem: # i3 config file (v4) bindsym Super_L exec notify-send "press" # or # bindcode 133 exec notify-send "press" bindsym --release Super_L exec notify-send "release" # or # bindcode --release 133 exec notify-send "release" Fixes #2733 --- src/bindings.c | 22 +++++++++++++--------- testcases/t/258-keypress-release.t | 24 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/bindings.c b/src/bindings.c index 823730ff6..38002396b 100644 --- a/src/bindings.c +++ b/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 @@ -271,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 - * also don’t return this binding, since it should not be executed - * yet (only when the keys are released). */ - bind = TAILQ_END(bindings); - break; + if (result) { + break; + } + continue; } /* 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)) { + if ((bind->release == B_UPON_KEYPRESS && is_release)) { continue; } - break; + 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; } /* diff --git a/testcases/t/258-keypress-release.t b/testcases/t/258-keypress-release.t index b92f723fd..766a8a1be 100644 --- a/testcases/t/258-keypress-release.t +++ b/testcases/t/258-keypress-release.t @@ -31,6 +31,11 @@ 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 EOT use i3test::XTEST; use ExtUtils::PkgConfig; @@ -110,6 +115,25 @@ is(listen_for_binding( ), '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'); + } done_testing; From dc0337d2e543c57f3b40bed5bb3f3a324718ab72 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Tue, 20 Mar 2018 01:44:50 +0200 Subject: [PATCH 4/4] Reset B_UPON_KEYRELEASE_IGNORE_MODS bindings when switching modes With example config: mode "a_mode" { bindcode 27 --release mode "default" } bindsym $mod+r mode "a_mode" The first time $mod+r is pressed "a_mode" is activated like normal. When r (bindcode 27) is pressed to exit the mode: - On the KeyPress event the corresponding bind->release is correctly marked as B_UPON_KEYRELEASE_IGNORE_MODS. - On the KeyRelease event the command 'mode "default"' is executed but bind->release is still B_UPON_KEYRELEASE_IGNORE_MODS since they are only reset on KeyPress events. The second time $mod+r is pressed and "a_mode" is activated and when the r key is released the 'mode "default"' is executed even though the mods are not matching since bind->release == B_UPON_KEYRELEASE_IGNORE_MODS. This still doesn't catch 2 cases: 1. When the order is: press $mod -> press r -> release $mod -> release r. Since 'r' is released without any modifiers the binding matches. 2. With: mode "resize" { bindsym --release r mode "default" } bindsym r mode "resize" This is arguably correct: on the KeyPress event we switch to the mode and on the KeyRelease we switch back. --- src/bindings.c | 8 ++++++++ testcases/t/258-keypress-release.t | 31 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/bindings.c b/src/bindings.c index 38002396b..228f7a9dc 100644 --- a/src/bindings.c +++ b/src/bindings.c @@ -649,6 +649,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")); diff --git a/testcases/t/258-keypress-release.t b/testcases/t/258-keypress-release.t index 766a8a1be..614164abf 100644 --- a/testcases/t/258-keypress-release.t +++ b/testcases/t/258-keypress-release.t @@ -36,6 +36,13 @@ bindsym --release Shift+x nop Shift+x # 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; @@ -134,6 +141,30 @@ is(listen_for_binding( '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;