microsoft
KBM - Keyboard Manager Shortcut Bugs
Summary of the Pull Request
With this PR, 2 bugs in KBM were fixed.
PR Checklist
- [ ] Closes: #34798 #34510
- [ ] Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
- [ ] Tests: Added/updated and all pass
- [ ] Localization: All end user facing strings can be localized
- [ ] Dev docs: Added/updated
- [ ] New binaries: Added on the required places
- [ ] JSON for signing for new binaries
- [ ] WXS for installer for new binaries and localization folder
- [ ] YML for CI pipeline for new test projects
- [ ] YML for signed pipeline
- [ ] Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx
Detailed Description of the Pull Request / Additional comments
KBM bugs are below:
- KBM shorcuts don't work when the PC is rebooted or Powertoys is restarted.
- When a shortcut and sending text with a common modifier key are invoked one after the other, KBM does not work.
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:
- Set KBM shortcuts.
- Close PowerToys.
- Press AltGR and dont release.
- Open Powertoys.
- Press Action keys. Check shortcuts don't work.
- Release all keys and press again shortcuts. Check this time shortcuts work properly.
Bug 2 Test Steps:
- Set keyboard layout to Italian (Italy)
- Set KBM shortcuts as above in screenshot.
- Press AltGr + I and check X is pressed.
- Release only I and press " \ " or " ' " . Check related text is sent. ( ■ , ... )
- All shortcuts above have same AltGr modifier keys. Test with combination of those shortcuts working properly.
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.
There were no updates to address the issues that I saw this PR created on my previous review. Also, @haoliuu tried to give a more recent review but it looks like the original issue no longer reproduces with the current state of the code. I advise verifying the original issue again and closing it if it no longer repros.
Unfortunately, I can still reproduce, the issue I've described in https://github.com/microsoft/PowerToys/issues/34798#issuecomment-2364041984 still present in 0.89.
