fix(keyboard): handle RDP Unicode input mode (Windows App for iPad)#10515
fix(keyboard): handle RDP Unicode input mode (Windows App for iPad)#10515beezzoo wants to merge 2 commits intowarpdotdev:masterfrom
Conversation
Fixes warpdotdev#6475 Windows App (Microsoft Remote Desktop) for iPad sends keyboard events in Unicode mode with no option to switch to Scancode mode. In this mode winit reports both physical_key and logical_key as Unidentified — only input.text carries the actual typed character. This broke all keyboard input in Warp over RDP: - Regular character input was silently dropped (convert_key returned None for Unidentified logical_key) - Modifier key tracking (left_alt_pressed/right_alt_pressed) never fired because it was gated on PhysicalKey::Code - ModifierKeyChanged events were never emitted for Shift/Ctrl/Alt - key_without_modifiers always returned None Fix: - Synthesize Key::Character from input.text when logical_key is Unidentified and text is present (RDP Unicode path) - Add fallback Alt-side tracking via Windows VK codes (0xA4/0xA5) from NativeKeyCode::Windows when physical_key is Unidentified - Add ModifierKeyChanged fallback using try_from_windows_vk() helper that maps VK_LSHIFT/VK_RSHIFT/VK_LCONTROL etc. to KeyCode - Fix get_key_without_modifiers to fall back to input.text lowercased Verified manually: regular typing, Ctrl+C/V/D shortcuts, Alt keys, arrows, Enter, Escape all work correctly over Windows App RDP from iPad.
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @beezzoo on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
github-actions
Bot
added
the
external-contributor
|
Signed as: @beezzoo |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds Windows-specific fallbacks for RDP Unicode keyboard input, including synthesizing character keys from input.text, mapping Windows VK modifier codes, and expanding keyboard tests.
Concerns
- Windows diagnostic logging is still present in the key-event path and logs
input.textfor every key press. temp.mdappears to be scratch verification notes and should not be committed.- The new RDP-focused tests mostly exercise
convert_keydirectly instead of the newUnidentified+input.textfallback path. - Manual testing is described, but no screenshot or recording is attached; include end-to-end evidence or explain why visual evidence is not possible for this keyboard workflow.
Security
- The diagnostic log can capture terminal input, including passwords, tokens, and commands, into stderr/log collection.
Verdict
Found: 1 critical, 1 important, 2 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| PhysicalKey::Code(c) => format!("Code({c:?})"), | ||
| PhysicalKey::Unidentified(n) => format!("Unidentified({n:?})"), | ||
| }; | ||
| eprintln!( |
There was a problem hiding this comment.
🚨 [CRITICAL] [SECURITY] Remove this diagnostic logging before merge; it records input.text for every Windows key press, which can capture passwords, tokens, and command input into stderr/logs.
| // Tests for RDP Unicode mode fallback: when physical_key is Unidentified, | ||
| // verify that convert_key still handles characters from logical_key correctly. | ||
| #[test] | ||
| fn rdp_unicode_mode_convert_key_handles_ascii_character() { |
There was a problem hiding this comment.
💡 [SUGGESTION] These tests bypass the new RDP fallback by constructing Key::Character directly; add coverage that drives the Unidentified + input.text path so regressions in this fix are caught.
| @@ -0,0 +1,9 @@ | |||
| 1. Build for Windows: cargo build --target x86_64-pc-windows-msvc -p warpui | |||
There was a problem hiding this comment.
💡 [SUGGESTION] Remove this scratch test-plan file from the PR; the checklist belongs in the PR description, not committed as temp.md.
beezzoo
force-pushed
the
fix/rdp-unicode-keyboard-input
branch
from
08c7299 to
14fc580
Compare
|
The diagnostic logging has already been removed before this PR was opened. The current commit contains no eprintln or logging of any kind — only the fix logic. Please re-review the latest commit (14fc580). |
Extract rdp_key_from_text() as a pub(super) function so tests can drive the exact code path executed when both physical_key and logical_key are Unidentified(Windows(_)) and input.text is Some. Add three new #[cfg(windows)] tests: - rdp_key_from_text_produces_lowercase_character: verifies text is lowercased before entering the pipeline (covers h, H, 1, !) - rdp_key_from_text_roundtrips_through_get_input_key_without_shift - rdp_key_from_text_roundtrips_through_get_input_key_with_shift: verifies that shift state is correctly re-applied after lowercasing
|
Added coverage in the new rdp_key_from_text_* tests (#cfg(windows)):
|
1 participant
Fixes #6475
Windows App (Microsoft Remote Desktop) for iPad sends keyboard events in Unicode mode with no option to
switch to Scancode mode. In this mode winit reports both physical_key and logical_key as Unidentified -
only input.text carries the actual typed character, causing all keyboard input in Warp to be silently
dropped.
Summary
Unicode path)
Test plan