fix(control): cross-platform clicks, keyboard shortcuts, and scroll direction - #47
Merged
Merged
Conversation
The host told the injection backend that its screen was however many pixels the
capture stream happened to be encoding. Those are different numbers in
different units, so remote clicks landed nowhere near where the guest aimed.
Worst on a Retina Mac. nut-js positions the pointer in logical points (1440x900
on a 2880x1800 panel) while the track reports physical pixels, so normalized
0.5 became (1440, 900) — the bottom-right corner — and everything past halfway
mapped clean off the display. Clicking did nothing at all.
It was wrong everywhere else too, just less visibly: the quality setting calls
applyConstraints({ width: { ideal: 1920 } }), so the track reports the *encoded*
resolution. A 2560x1440 Linux host was treated as 1920x1080, putting every
click at 75% of its intended offset and getting worse toward the bottom right.
Normalized coordinates need nothing but the host's own screen geometry, and
each backend already reads that from its own OS API in that API's units —
nut-js from screen.width(), the Wayland backend from the compositor. So the
renderer now sends no screen size at all, and the test that asserted it did is
inverted to keep it that way.
Modifiers went over the wire exactly as the viewer's OS reported them, which does not travel. "The shortcut key" is Cmd on macOS and Ctrl everywhere else, so both directions of a mixed session were broken: - a Mac viewer's Cmd+C arrived at a Linux host as Super+C, which copies nothing - a Linux or Windows viewer's Ctrl+C arrived at a macOS host as Control+C, which also copies nothing Copy, paste, save, undo, quit — every shortcut, in both directions. Viewers now send `accel` for "my platform's shortcut modifier was held", and the host maps that to whichever modifier means shortcut locally. The literal `ctrl` and `meta` still travel for the cases that really do mean Control (macOS Control+click) or Super (Linux window manager bindings), so Control+Cmd+F still arrives intact on a Mac host. The field is optional: an older viewer that omits it keeps today's literal pass-through rather than losing modifiers altogether. Split across packages because remote-input is deliberately standalone and Node-only: `modifiersFromDomEvent` (viewer, DOM-facing) lives in shared-types, `resolveModifiers` (host) in remote-input. Both are pure and tested, including the two cases above. Platform detection is its own tested unit per app rather than inline in the hook — the desktop reads it from preload, the web app from the user agent because a browser has nothing better.
Both backends treated a positive deltaY as a scroll up. In the DOM, positive deltaY is a scroll *down*, so every remote scroll went the wrong way — on every platform, since nut-js and ydotool had the same inversion. Horizontal was already correct. Two tests asserted the inverted behaviour and have been corrected rather than deleted; the convention is now written down on MouseScrollEvent.deltaY, which is where the ambiguity that caused this belonged in the first place.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const backend = new WaylandYdotoolInputBackend(run, { | ||
| hasBinary: true, | ||
| hasSocket: true, | ||
| socketPath: '/tmp/.ydotool_socket', |
ThreatCrush Security Scan160 finding(s) HIGH/CRITICAL: 5 | MEDIUM: 49 | LOW: 106
…and 110 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The merge of #46 went in at the wrong head — these four commits were pushed
after the PR was created and didn't make it into master.
What's here
Remote clicks mapped with the host's screen, not the stream's resolution.
The renderer was feeding the capture track's pixel dimensions as the injection
screen size. Those are in different units: on a Retina Mac, nut-js positions
in logical points (1440x900) while the track reports physical pixels
(2880x1800), so normalized 0.5 landed at the bottom-right corner and clicking
did nothing at all. Wrong on every platform, just less visibly — the quality
setting calls
applyConstraints({ width: { ideal: 1920 } }), so the trackreports the encoded resolution and every click landed at 75% of its offset.
The backend already reads the host's own geometry from the OS in that API's
units, so the renderer no longer sends a screen size.
Keyboard shortcuts survive crossing operating systems. Modifiers went
over the wire exactly as the viewer's OS reported them — a Mac viewer's Cmd+C
arrived at a Linux host as Super+C (copies nothing), and a Linux viewer's
Ctrl+C arrived at a macOS host as Control+C (also copies nothing). Copy,
paste, save, undo, quit — every shortcut broke in both directions. Viewers
now send
accelfor "my platform's shortcut modifier", host maps to its own.Scroll direction corrected. Both backends treated positive
deltaYasscroll up; DOM convention is positive = scroll down. Every remote scroll went
the wrong way on every platform.
Validation: lint, typecheck, 1447 tests, 34 script tests, format, build — all green.