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

input: Send modifiers of modifier-only bindings to clients #658

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

Drakulix
Copy link
Member

I feel like this is the better approach rather then pretending the modifier event never happened. Potentially fixes missed inputs and cleans up this method a bit.

@ids1024
Copy link
Member

ids1024 commented Jul 29, 2024

Testing on Gnome, it seems to not send the super key press event. But then if it's followed with another key that doesn't have a binding, it sends modifiers first.

...but it doesn't actually end up sending the key press event for Super_L, while it does send the key release event for it, so Gnome doesn't seem to be doing this correctly either (based on the version in 24.04).

Overall I don't know it matters much whether or not we suppress the modifier, as long as make it behave consistently. (Send both key press and release, or neither; probably send a modifiers change if and only if the key press/down are sent.)

@ids1024
Copy link
Member

ids1024 commented Jul 29, 2024

Testing this branch, it does seem to behave consistently and correctly (more so than Gnome) though maybe there are some edge cases I haven't tried.

@Drakulix
Copy link
Member Author

Testing on Gnome, it seems to not send the super key press event. But then if it's followed with another key that doesn't have a binding, it sends modifiers first.

...but it doesn't actually end up sending the key press event for Super_L, while it does send the key release event for it, so Gnome doesn't seem to be doing this correctly either (based on the version in 24.04).

That is exactly what I wanted to avoid with this.

Sending the pressed event later would also work, but feels much more annoying to track and to get right (as we have to figure out the correct keycode), which is precisely why I opted for this as most application will probably not react to a modifier being pressed anyway. And for those that do, I can see how filtering that might even be more confusing to users.

@ids1024
Copy link
Member

ids1024 commented Jul 29, 2024

Yeah. Deferring sending the modifier would probably be more confusing to users if the modifier impacts an application in some way. So it doesn't really sound better in practice even if it theoretically makes sense.

@Drakulix Drakulix marked this pull request as ready for review July 29, 2024 17:19
Copy link
Member

@ids1024 ids1024 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't confirmed any issues that this may fix in clients, but the changes here look good, and the behavior seems correct based on wev.

@Drakulix Drakulix merged commit af553a1 into master Jul 29, 2024
7 checks passed
@Drakulix Drakulix deleted the send-bound-modifiers branch July 29, 2024 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants