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

x11: Don't panic when using dead keys #432

Merged
merged 1 commit into from
Apr 5, 2018

Conversation

francesca64
Copy link
Member

Fixes #366

I was originally planning to fix this by rewriting keyboard handling to use XInput2+XKB+libxkbcommon, and I do indeed have a fully functional implementation of that. However, I never managed to figure out how to handle IME, and I could seldom find the motivation to try to conquer yet another dimension of X11 eroticism. That only got worse when I discovered older libxkbcommon versions, which are still in use, don't support compose sequences. As a result, this fairly important issue has gone unresolved for much longer than I ever intended.

This PR fixes the issue using a bold new solution: a single if that checks if the keycode is non-zero. A KeyPress with a keycode of 0 is received at the end of compose sequences and pre-edit sequences, which has our desired UTF8 string associated with it. The panic was simply the result of trying to send a KeyboardInput event for this input, which involved trying to calculate a scancode for this nonexistent key. So, now we just don't try to send a KeyboardInput event in that case, and just like that, everything works.

I should note that the behavior of the X11 backend isn't consistent with the expectations laid out in #343. While I was able to easily achieve that behavior using XInput2, vanilla X11 seems less cooperative. For individual keys pressed during a compose/pre-edit sequence, X11 only generates KeyRelease events, so the user receives KeyboardInput events for key releases with no corresponding press events. In the case of compose, the ReceivedCharacter is sent to the user before the KeyboardInput for the release of the final key in the sequence. For pre-editing, a KeyboardInput for the release of the return key is sent after the ReceivedCharacter.

@b-r-u
Copy link
Contributor

b-r-u commented Mar 24, 2018

Thanks for wandering through the arcane landscape of X11 APIs!
Despite the drawbacks from your last paragraph, this looks like a really simple solution to the problem.

@Ralith
Copy link
Contributor

Ralith commented Mar 24, 2018

However, I never managed to figure out how to handle IME

If it's any consolation, that's why I gave up on xkbcommon-ifying the main keyboard input code path myself. It may simply be impossible to support anything other than XCompose with that approach, which I imagine eastern users would be rather unsatisfied with. At this point I'm 100% approving of bodging X11 support into something approximately sane and focusing on Wayland for doing it Right™.

Copy link
Contributor

@tomaka tomaka left a comment

Choose a reason for hiding this comment

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

Thanks. I guess it's better this kind of hack than things not working. Also needs a changelog entry.

@francesca64 francesca64 force-pushed the x11-dead-keys branch 3 times, most recently from ac08c3e to 75d16ec Compare March 29, 2018 18:37
@francesca64
Copy link
Member Author

I wouldn't say this is a hack, since as far as I can tell, this is the correct way to implement this. As outlined here, the composed character is injected using a "fake" KeyPress with a keycode of 0, and all this PR does is prevent us from treating it like an ordinary KeyPress.

Anyway, I also solved the problems outlined in my last paragraph! The X11 backend now fully conforms to #343, thanks to some changes in how events are filtered.

@francesca64
Copy link
Member Author

The event filtering hack unfortunately had to go. It resulted in backspace and arrow key inputs being doubled, so the original proviso once again applies.

@francesca64 francesca64 merged commit f3ab8af into rust-windowing:master Apr 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Deadkeys on Linux panics
4 participants