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

Figure out VK_PACKET/ConsoleKey.Packet and build support for it #3119

Open
4 tasks
tig opened this issue Jan 4, 2024 · 0 comments
Open
4 tasks

Figure out VK_PACKET/ConsoleKey.Packet and build support for it #3119

tig opened this issue Jan 4, 2024 · 0 comments
Labels
design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...)
Milestone

Comments

@tig
Copy link
Collaborator

tig commented Jan 4, 2024

Last year a developer submitted an Issue where key presses were not being handled correctly when using a Terminal.Gui app over RDP (#2008). In investigating that issue @BDisp and @tznind learned about ConsoleKey.Packet.

We learned that ConsoleKey.Packet meant:

Used to pass Unicode characters as if they were keystrokes. The VK_PACKET key is the low word of a 32-bit Virtual Key value used for non-keyboard input methods. For more information, see Remark in KEYBDINPUT, SendInput, WM_KEYDOWN, and WM_KEYUP"

The only OS that knows what VK_PACKET is, is Windows.

In the fix (#2032) a whole bunch of infrastructure code was introduced to try to decode these packets to solve for this use-case (users using Terminal.Gui apps over RDP).

In addition to the infrastructure code (ConsoleKeyMapping.DecodeVKPacketToKConsoleKeyInfo etc...) a Scenario was developed to try to simulate things so this code could be better tested (VkeyPacketSimulator).

As far as I can tell, none of the core Terminal.Gui maintainers regularly test the VK_PACKET support in a real scenario (e.g. over RDP). No-one has yet to provide a reproducable real-world use case that anyone on the team can use.

  1. The use-case we are discussing is completely tied to Windows. The only way a ConsoleDriver will ever get a key event with ConsoleKey.Packet is if the user is running their app on Windows and using that app over RDP (there may be other cases but nobody's articulated one precisely). The only ConsoleDrivers that will ever get a key event with ConsoleKey.Packet are WindowsDriver and NetDriver.

  2. The infrastructure code discussed above is inherently flawed. Specifically: HashSet<ScanCodeMapping> _scanCodes. It assumes it is possible to have a singular map of scancodes, virtual key codes (VK_), and shift modifiers to characters that works across all keyboard layouts. It, effectively, tries to duplicate what the Win32 method MapVirtualKeyCode does in a very simplistic way (that will never work).

  3. By extension to 2) above, the VkeyPacketSimulator is inherently flawed and doesn't do what it was intended to do (provide a way of testing VK_PACKET handling in WindowsDriver and NetDriver).

  4. Not directly related to VK_PACKET support, but relevant is the fact that WindowsDriver and NetDriver have always been flawed in how they handled keys like Oem3. The only correct way to handle those keys is to use MapVirtualKeyCode (this PR addresses this).

The correct solution to all of this is:

a) Create clear and easy-to-use instructions for setting up a system to routinely test Terminal.Gui over RDP (and any other use-case where VK_PACKET key events are generated).
b) Change WindowsDriver to use MapVirtualKeyCode when key events with VK_PACKET are received. Most of the code in ConsoleKeyMapping can be thrown away. #3078 addresses some of this.
c) Built a better testing API into ConsoleDriver that lets us simulate VK_PACKET key events in unit tests such that both WindowsDriver and NetDriver are better tested.
d) Refactor the VkeyPacketSimulator to use c), so we can have a UI-based tester.

I have a bad memory, and I apologize this didn't come to me before. But in 2004-ish I was partially responsible for adding VK_PACKET support to Windows. I had created the concept of Windows Media Center Extenders (Bobsled) and we were in deep collaboration with the RDP team because I had the vision that Extenders would use RDP as their protocol. All UI could run on the Media Center PC an be remoted to the Bobsled device as GDI primitives via RDP. We needed two features RDP didn't have at the time:

  • The ability to have video streams be sent to the RDP client out-of-band of the GDI streaming.
  • The ability to have commands from the IR-based MCE remote control be sent out-of-band of the GDI streaming.

The RDP team had woken up to the fact that RDP didn't support Unicode and varying keyboard layouts well. Hence VK_PACKET was created (and the other Media Center specific VK_ keys such as VK_MEDIA_NEXT_TRACK.

I hope that there's a similar API to MapVirtualKeyCode (or GetKeyboardLayout) on the Mac and Linux. There has to be. And it's possible the keyboard coding used maps well to a lot of Windows VK_ defines. There may even be a key event that is similar to VK_PACKET where all you get is the unicode char, which would be sweet. But that is a separate discussion.

For this Issue to be addressed:

  • Create clear and easy-to-use instructions for setting up a system to routinely test Terminal.Gui over RDP (and any other use-case where VK_PACKET key events are generated).
  • Change WindowsDriver to use MapVirtualKeyCode when key events with VK_PACKET are received. Most of the code in ConsoleKeyMapping can be thrown away. Cleans up key handling in drivers (was "fixes VkeyPacketSimulator is broken") #3078 addresses some of this.
  • Build a better testing API into ConsoleDriver that lets us simulate VK_PACKET key events in unit tests such that both WindowsDriver and NetDriver are better tested.
  • Refactor the VkeyPacketSimulator to use c), so we can have a UI-based tester.

For now, the VkeyPacketSimulator, much of the code in ConsoleKeyMapping, and most of the tests in ConsoleKeyMappingTests.cs are invalid/useless and should be ignored (or deleted)>

@tig tig added the design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...) label Jan 4, 2024
@tig tig added this to the v2 Release milestone Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...)
Projects
Status: No status
Status: 🆕 Not Triaged
Development

No branches or pull requests

1 participant