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

Windows {#Alt_L(Escape)} confuses the application #1154

Closed
user202729 opened this issue Oct 22, 2020 · 4 comments · Fixed by #1274
Closed

Windows {#Alt_L(Escape)} confuses the application #1154

user202729 opened this issue Oct 22, 2020 · 4 comments · Fixed by #1274

Comments

@user202729
Copy link
Member

Reproducing

  • Define some strokes (say X) to translate to {#Alt_L(Escape)}{}{^} (which is "switch focus to next application" keyboard shortcut in Windows)
  • Open a cmd window and a notepad window. Type something into notepad. (although the issue happens for almost all windows, it's easy to demonstrate with notepad)
  • Switch focus to notepad (with the mouse)
  • Switch focus to cmd
  • Stroke X. Focus should be switched to notepad now.
  • Press Control+A (with a normal keyboard)

Observe that notepad doesn't select all text.

If the left alt button (on any keyboard connected to the computer) is pressed and released, then control+A starts to work again in notepad.

Plover Version

Latest master.

System

Windows 10.

@user202729
Copy link
Member Author

I spent some (a lot of) time debugging the issue, and found out that the issue is with the KEYEVENTF_EXTENDEDKEY flag.

  • The set of extended keys in Plover source code is completely wrong. The correct set should be (approximately) (source)
e0 1c (Keypad Enter)
e0 1d (RCtrl)
e0 2a (fake LShift)
e0 35 (Keypad-/)
e0 36 (fake RShift)
e0 37 (Ctrl-PrtScn)
e0 38 (RAlt)
e0 46 (Ctrl-Break)
e0 47 (Grey Home)
e0 48 (Grey Up)
e0 49 (Grey PgUp)
e0 4b (Grey Left)
e0 4d (Grey Right)
e0 4f (Grey End)
e0 50 (Grey Down)
e0 51 (Grey PgDn)
e0 52 (Grey Insert)
e0 53 (Grey Delete)
  • AutoHotKey doesn't have the issue, and it sends the "correct" hardware scan code together with the KEYEVENTF_EXTENDEDKEY flag off.
    (it uses keybd_event instead of SendInput, but their behavior is the same in this case)
  • Plover always use 0 for scan code anyway (with non-Unicode characters), and so far there's no problems with that. According to this post and this post, most programs will work well despite the incorrect scan code. Besides, shift_l and shift_r have separate virtual key codes.
  • If the flag KEYEVENTF_EXTENDEDKEY is always off, then the issue doesn't happen. I'm not sure if the patch creates any other issues.

@user202729
Copy link
Member Author

With some more testing I've determined that:

  • On keycode.info on Firefox, Control_r is only correctly determined if both the scan code (0x1d) and the extended flag (must be set) is correct.
  • Otherwise, removing the extended flag fixes this particular issue.
  • Currently Plover always use 0 for the scan code, I don't think there's any value in making the extended flag correct.
  • The original value of the EXTENDED_KEYS set comes from Rewrite keyboard emulation for Windows #213 with no explanation. @morinted ?

@TheaMorin
Copy link
Member

I tried to do some searching on my rational, came up empty.

I think one of the concerns with that branch was trying to support Scrivener or kdiff3.

From the second post you linked, this is an interesting answer, and I wonder if we can test whether we're properly able to distinguish between numlock and pause?

But yes, as we don't send the scan code, it's probably useless. It's a question as to whether setting the scan code would be valuable to extend support.

@benoit-pierre
Copy link
Member

The official documentation is clear, the flag should only be set when sending scan codes. I agree that we might consider switching to sending scan codes over virtual codes to improve compatibility with some applications, but for now, we should not set it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants