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

fix backspace and navigation keys with caps lock on #687

Closed
wants to merge 1 commit into from

Conversation

groverlynn
Copy link
Contributor

Pull request

Issue tracker

Fixes will automatically close the related issue

Fixes #

Feature

Fix bugs where backspace and navigation keys are not correctly interpreted when caps lock is on. This is particularly problematic in non-inline mode: letters and punctuators are correctly inputed into rime while control actions happen outside of rime directly in the text area of the client app. This pull simply dismisses cap lock as a modifier except when it's used to toggle ascii mode (capitalization is done by system before feeding into rime).

Unit test

  • Done

Manual test

  • Done

Code Review

  1. Unit and manual test pass
  2. GitHub Action CI pass
  3. At least one contributor reviews and votes
  4. Can be merged clean without conflicts
  5. PR will be merged by rebase upstream base

Additional Info

@lotem
Copy link
Member

lotem commented Oct 15, 2023

Need more work:
I don't like how the fix is done. It is surprising that a getter function has some hidden logic.

@groverlynn
Copy link
Contributor Author

Need more work: I don't like how the fix is done. It is surprising that a getter function has some hidden logic.

But this is exactly how Capslock is defined as a unique modifier: it is categorized as modifier but it cannot function like a normal modifier such that modifies main keys. So, unless you create a designate category for Capslock, I don't see any other way to fix this.

@lotem
Copy link
Member

lotem commented Feb 8, 2024

CapsLock is a valid modifier. KeyEvent::modifier() should include Caps Lock.
It's up to the user to decide whether they want to use the modifier.

@lotem lotem closed this Feb 8, 2024
@groverlynn
Copy link
Contributor Author

CapsLock is a valid modifier. KeyEvent::modifier() should include Caps Lock. It's up to the user to decide whether they want to use the modifier.

@lotem you can check others' codes, say qt, they all apply the same technique: capslock (and numlock on win for that matter) is considered modifier on its own, but not when other modifiers are present. Because they are "lock" modifiers, meaning they are present if lock is on, not when pressed like other modifiers. And that's why you only get key down for capslock and numlock, no key up

@lotem
Copy link
Member

lotem commented Apr 30, 2024

I agree with your statement that caps lock is an independent modifier. That's the precise reason why I find it unacceptable when the modifiers() getter function silently discards it instead of returning it like other modifiers.

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