Hitting keys in the wrong order causes keys to be infinitely re-emitted

[TTWNO]

Version Information:

• Linux blueie 6.0.2-zen1-1-zen #1 ZEN SMP PREEMPT_DYNAMIC Sat, 15 Oct 2022 14:00:51 +0000 x86_64 GNU/Linux
• swhkd 1.2.1

Describe the bug:

If keys are pressed in the reverse order specified in the config file, the event will still be triggered. In addition, the first key in the sequence will be repeated infinitely.
For example, if setting up the key combo like so:

ctrl + p
    spd-say 'print me'

When pressing the key combination p + ctrl, the ctrl + p command will be run. After the command is run, the p key will continue to type itself infinitely.

Expected behavior:

The ctrl + p command should not be triggered by a p + ctrl key combo. In addition, the p key should not continue to type itself infinitely.

Actual behavior:

See above.

To Reproduce:

Setup a config file like the following:

ctrl + p
    spd-say 'hello'

Now start swhkd and swhks normally:

swhks &
pkexec swhkd

Now attempt to use the correct ordering of the key combo: ctrl + p; you should hear your system speak the work "hello". Conversely, try to press the keys in the opposite order p + ctrl, and notice what happens.
First, the command will still be triggered.
Second, your p key will now type itself over and over and over again until you kill the process.

Additional information:

N/A

[TTWNO]

I'm not exactly sure what even causes this issue, but I'm quite curious if you could point me in the right direction. We do have a fork for our Odilia project in our repo for reference, and I'd be happy to make a PR for this issue since it is of immediate effect to us. I just can't quite fathom where the error is coming from, but perhaps that's just me not paying attention to the code very closely.

Any help, even just a hint of what to look into to get started on fixing this bug would be much appreciated.

Thanks for your incredible work, y'all!

[ajanon]

Thanks for the report!

I think p+ctrl being triggered exactly like ctrl+p is by design, take a look at:

swhkd/swhkd/src/daemon.rs

Lines 341 to 343 in fdf5e10

	if (!keyboard_state.state_modifiers.is_empty() && hotkey.modifiers().contains(&config::Modifier::Any) || keyboard_state.state_modifiers.iter().all(|x| hotkey.modifiers().contains(x))
	&& keyboard_state.state_modifiers.len() == hotkey.modifiers().len())
	&& keyboard_state.state_keysyms.contains(hotkey.keysym())

Basically, this checks that all modifier keys are pressed and that the keysym is also being pressed.
The keysym is added to the list of currently pressed keys at:

swhkd/swhkd/src/daemon.rs

Lines 277 to 284 in fdf5e10

	// Key press
	1 => {
	if let Some(modifier) = modifiers_map.get(&key) {
	keyboard_state.state_modifiers.insert(*modifier);
	} else {
	keyboard_state.state_keysyms.insert(key);
	}
	}

Is triggering the hotkey on p+ctrl really a problem, though? To me, it seems like the expected behavior. If you think this should be different, do you have a use case in mind where this could be an issue?

The second part of your issue seems pretty bad still. I will take a look as soon as possible.

[ajanon]

Unfortunately, I cannot reproduce the issue. I used a slightly different config to test your issue, on the latest git version of swhkd:

ctrl + p
    notify-send Test

Pressing p + ctrl does trigger the hotkey, but it has no issue stopping afterwards.

Could you give a bit more info about your environment?

[ajanon]

Never mind, I managed to trigger the bug you mentioned. I will investigate what's happening.

[ajanon]

Here are my findings:

When first pressing a key (p in the example) without a modifier, a key press event is sent to the virtual device to be consumed by other libraries and applications. Pressing the modifier key afterwards triggers the hotkey as expected. Releasing only the key at this point does not send a key release event to the virtual output, as the event_in_hotkeys boolean is true (control is still being pressed and control + p is mapped to a hotkey after all). To any library afterwards, the key is still being pressed as no release event has been received.

To avoid killing the daemon during testing, you can just tap the key (without any modifier) to trigger a key press and key release event.

[TTWNO]

The issue here is that, for example, p + ctrl does not work for most applications when they provide a key combo. Take for example, Firefox, which allows you to print a webpage with the ctrl + p key combo. If I type p + ctrl, it types the letter p out immediately, then control does nothing, because p + ctrl is not a key combo according to Firefox.

Due to that behavior in other applications, it makes the "reverse key combo still being triggered" an issue, since now the p press event will be taken at face value by the application, but as soon as its part of the binding and not released, the application has no idea that the key is now being used as part of a binding so it continues to type it nonstop.

I don't see a way to combine solving this issue with the current architecture for the following reason: We cannot send out a key release event for every key in a pressed key binding; this could cause even more weird edge cases. (Side note: I don't know why an application would do something on a key release instead of a press, but it would certainly break those theoretical applications to have key bindings sending false events like this.)

If the key bindings were to be ordered, where ctrl + p != p + ctrl, it will adhere to how most other applications have keybindings set up, thus automatically solving the second issue outright.

Unless you can think of a better solution, I think ordered key bindings make the most sense here. It solved the other issue I'm having and adheres more closely to how most other applications handle keybindings.

Thoughts?

[ajanon]

Could you please test branch key_release_events_fix (in PR #167) and tell me if it solves your issue? (at least the second part).

As for ordered key bindings: making modifiers ordered might be hard (and I would find this pretty unintuitive to use personally), but triggering a hotkey only if the latest key press (/release) event is from a non-modifier key might be feasible.

We could also add new syntax for the config to tell that a keybind must be ordered. For instance:

# Trigger only if keys are pressed in the order given
>super + ctrl + p
   spd-say 'hello'

Or we could make ordered keybinds the default and unordered a special case. I think it would be better to keep it unordered by default to avoid breaking users' existing workflows and expectations.

[TTWNO]

Checked out the branch. This does fix the issue of the key perpetually repeating itself even after the key is released. However, now the key combo is passed through, in full, to the underlying application as if I had pressed the keys the other way around.

For example, in Firefox, pressed ctrl + p will activate the key combo and not be passed through to Firefox. Awesome! Pressed p + ctrl, however, causes Firefox's print page dialog to appear as well as triggered the binding in swhkd.