-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
KBM - Keyboard Manager Shortcut Bugs #35201
base: main
Are you sure you want to change the base?
KBM - Keyboard Manager Shortcut Bugs #35201
Conversation
…eset after ResetPreviousModifierKey.
🔥🔥🔥 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't repro the original issue for Bug 1 with the instructions in the PR text in 0.85.1.
I did try something that seems to have gotten worse, though.
With the AltGr+I to Shift(left)+X mapping, try the following:
1 - Use the mapping, don't release Alt Gr
2 - Disable KBM
3 - Enable KBM
4 - Try using the mapping again without having released Alt Gr.
5 - release Alt Gr
6 - try the mapping again
On .85.1, Shift(left) gets stuck. On this PR, both Shift(left) and Ctrl(left) get stuck.
Hi @jaimecbernardo, I added Bug 1 improvements as a precaution for users who press modifier keys while KBM has not started yet. The case you tested is different from this one. When KBM is disabled before the key up event occurs, Shift left remains stuck because it is in the shortcut(Shift Left) in version 0.85.1. This was already what caused the bug in Bug 2. To solve this, I kept the pressed modifier keys (altgr) pressed instead of the invoked shortcut(shift left) modifier key. Alt gr is a special key and I make special adaptations to it in KBM issues. There is the "isAltRightKeyInvoked" variable that I use for this. In the code, there are special cases where if this bool variable is true. When the code is restarted as in your test case, although Altgr is pressed, I cannot send the Left ctrl up event because this variable is false. I tried the same steps with Ctrl(Left)+I to Shift(left)+X and the problem did not occur. Finally, the purpose of this PR is different. Bug 1 changes was made to prevent errors that occur when modifier keys are pressed before starting KBM. Bug 2 changes was made for the errors that occur when trying to send a shortcut and send text with common modifier keys at the same time with not releasing that modifier keys. Disabling KBM while keys are pressed is a different issue. Only difference is keys pressed instead of the set shortcut become stuck in this PR. Also, if you disable KBM while holding down the modifierkey and action key in both 0.85.1 and this PR, you will see that the Shift Left and x keys are stuck. The bug you mentioned may appear as follows. If KBM crashes while the user is invoking a shortcut using altgr and alt gr is pressed in that time. But I think the probability of this issue to be very low. My suggestion is to fix these two existing bugs now. Because both have been frequently reported. If such an issue reported from users, we will try to find a special solution for it like reset all pressed keys when KBM started. |
Summary of the Pull Request
With this PR, 2 bugs in KBM were fixed.
PR Checklist
Detailed Description of the Pull Request / Additional comments
KBM bugs are below:
Validation Steps Performed
Shortcut mappings for testing are as follows.
Note 1: Shortcut mappings are with AltGr modifier key. Also tested with only Ctrl Left with the Turkish Keyboard Layout or only with Alt Right with the US keyboard layout.
Note 2: Tested with Italian (Italy) keyboard layout.
Bug 1 Test Steps:
Bug 2 Test Steps: