Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

keysym --release is unreliable #6456

Open
Spferical opened this issue Aug 21, 2021 · 5 comments · May be fixed by #6616 or #7602
Open

keysym --release is unreliable #6456

Spferical opened this issue Aug 21, 2021 · 5 comments · May be fixed by #6616 or #7602
Labels
bug Not working as intended

Comments

@Spferical
Copy link

  • Sway Version: 1.6.1

  • Description:

    • Configure bindsym --release Alt_R notify-send foobar
    • Press Alt_R key
    • Press another key, e.g. Alt_L
    • Release Alt_R

If any key is pressed before releasing the configured key, the bindsym command is not executed. The same bug happens if I bind to e.g. a letter key.

According to the discussion here, a similar issue was fixed back in 2018, so this might be a regression. My use case is that I would like to configure a push-to-talk button through sway that mutes the microphone when let go.

@Spferical Spferical added the bug Not working as intended label Aug 21, 2021
@k3d3
Copy link

k3d3 commented Oct 15, 2021

I get the same thing.

I use bindsym for my Mumble push-to-talk, and my config contains

bindsym --no-repeat --whole-window           BTN_SIDE       exec mumble rpc starttalking
bindsym --no-repeat --whole-window --release BTN_SIDE       exec mumble rpc stoptalking

So it uses the button on the side of my mouse. If I hold that button down, left-click the mouse, then release that side button, Mumble keeps my mic activated.

@k3d3
Copy link

k3d3 commented Oct 17, 2021

I'm looking through the code, and it appears my problem comes down to how modifiers are handled in bindings.

I don't specify any modifiers with my button press, so if I'm holding down shift when I release the mouse button, the binding does not match.

I believe the issue comes down to this line

if (modifiers ^ binding->modifiers ||
where it checks if the modifier state is exactly the same as the binding.

Is this the right way to do it? Shouldn't we mask instead, so that a binding that accepts shift works if you're holding down both shift and ctrl, for example? Or that any set of modifiers is allowed if the binding doesn't specify any?

k3d3 added a commit to k3d3/sway that referenced this issue Oct 17, 2021
Fixes swaywm#6456.

Using XOR for checking modifiers means the full modifier state must be
exactly the same. For example, a binding that requires Shift won't
trigger if Shift *and* Ctrl are being pressed.

Instead, using masking means a binding requiring Shift will be triggered
if Shift is pressed, even if Ctrl is also pressed at the same time.
k3d3 added a commit to k3d3/sway that referenced this issue Oct 17, 2021
Fixes swaywm#6456.

Using XOR for checking modifiers means the full modifier state must be
exactly the same. For example, a binding that requires Shift won't
trigger if Shift *and* Ctrl are being pressed.

Instead, using masking means a binding requiring Shift will be triggered
if Shift is pressed, even if Ctrl is also pressed at the same time.
k3d3 added a commit to k3d3/sway that referenced this issue Oct 17, 2021
Closes swaywm#6456.

The i3 window manager ignores modifiers when a button is being released.
Mimic this behaviour in Sway.
@k3d3
Copy link

k3d3 commented Oct 17, 2021

As far as pushing keys, it appears this is intended i3 behaviour, however releasing keys ignores modifiers.

So combined with #6616, the best way to solve this is to (unfortunately) spam the sway config with every combination of modifier keys that you commonly press. For my mumble PTT use-case:

bindsym --no-repeat --whole-window BTN_SIDE                 exec mumble rpc starttalking
bindsym --no-repeat --whole-window Ctrl+BTN_SIDE            exec mumble rpc starttalking
bindsym --no-repeat --whole-window Shift+BTN_SIDE           exec mumble rpc starttalking
bindsym --no-repeat --whole-window Mod4+BTN_SIDE            exec mumble rpc starttalking
bindsym --no-repeat --whole-window Ctrl+Shift+BTN_SIDE      exec mumble rpc starttalking
bindsym --no-repeat --whole-window Ctrl+Mod4+BTN_SIDE       exec mumble rpc starttalking
bindsym --no-repeat --whole-window Shift+Mod4+BTN_SIDE      exec mumble rpc starttalking
bindsym --no-repeat --whole-window Ctrl+Shift+Mod4+BTN_SIDE exec mumble rpc starttalking

It's not pretty, but it works.

@k3d3
Copy link

k3d3 commented Oct 30, 2021

@emersion If I were to create a PR that adds a new flag to bindsym, maybe --mask-modifiers, that changes the behaviour to what I had explained before (pressing a button ignores the state of modifiers not mentioned in the binding, i.e. matching a modifier subset), would this be considered for merging?

@emersion
Copy link
Member

Unlikely, in general we're not adding new features unless i3 has them.

lostmythread pushed a commit to lostmythread/sway that referenced this issue Mar 27, 2022
This commit changes the "release binding" behavior to ignore foreign
keypresses (pressed keys that are not part of a given release binding's
definition) while still triggering the bound action when the original
key from the binding is released. This, unless foreign keypresses match
a different release binding, in which case the latter binding takes
over.

For instance, assuming a release binding is configured for <Mod>, a key
corresponding to one of the possible modifier keys. The previous behavior
for the following sequence was to trigger the action only for the following
sequence:

- <Mod>(press), <Mod>(release)

but *not* in the following case:

- <Mod>(press), <Tab>(press), <Tab>(release), <Mod>(release)

With this commit both sequences trigger the bound action.

This is useful to control external software by triggering possibly
repeated actions, e.g. to navigate windows using <Mod> + <Tab>
and triggering a final action when the <Mod> key is released (e.g.
hiding a UI, reactivating LRU accounting, ...)

Closes swaywm#6456
@chayleaf chayleaf linked a pull request May 23, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Not working as intended
3 participants