fix(control): map clicks to the picture, not the letterboxed element around it - #61
Merged
Merged
Conversation
…around it The remote screen is rendered `object-contain`, so the browser letterboxes it whenever its aspect ratio differs from the viewer's window: the <video> element fills the container, but the picture is centred inside it with dead space on two sides. Coordinates were normalized against the element. Every click therefore carried an offset and a scale error, both proportional to how much dead space there was. A 16:9 host screen watched in a square-ish window letterboxes to a fifth of the height in bars — so a guest clicking the very top of the host's screen, reaching for a menu bar, put the pointer a fifth of the way down it instead. Open a side panel and it gets worse; happen to match the host's aspect ratio and it looks perfect. That combination is why this reads as flaky rather than broken. The geometry already existed and was already correct: `getContainRect` was written for the camera bubble, which had to be positioned against the picture for exactly the same reason. It just was not used by the input path. It moves to @pairux/shared-types so there is one implementation, since an overlay drawn against one rectangle and clicks mapped against another is its own bug. Three places were reading the wrong rectangle: - Click and move coordinates, in both viewers. This is the one that made control feel unusable. - Pointer-lock movement, which scaled deltas by the element, so the virtual pointer crossed one axis too slowly by whatever fraction the bars took. - The remote cursor overlay, which drew the guest's cursor by percentage of the container. That one is worse than it sounds: the cursor and the click disagreed, so the host watched a pointer hover over one thing and click another. Positions inside the letterbox bars clamp to the nearest edge of the picture rather than being dropped, for the same reason coordinates are clamped everywhere else — the screen edges have to stay reachable. The web test setup gains a ResizeObserver stand-in, which jsdom does not implement and the overlay's measurement needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan169 finding(s) HIGH/CRITICAL: 16 | MEDIUM: 47 | LOW: 106
…and 119 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
1 participant
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.
Found while auditing the remote-control path end to end after #59. This one has a wider blast radius than anything in that PR: it affects every click in every session, on both viewers.
The bug
The remote screen renders
object-contain, so the browser letterboxes it whenever its aspect ratio differs from the viewer's window — the<video>element fills the container, but the picture is centred inside it with dead space on two sides.Coordinates were normalized against the element:
So every click carried an offset and a scale error, both proportional to how much dead space there was.
Concretely: a 16:9 host screen watched in a 1000x1000 area letterboxes to 1000x562.5, with 218.75px bars top and bottom. A guest clicking the very top of the host's screen — reaching for a menu bar, the single most common thing to aim at — put the pointer at
y = 0.219, a fifth of the way down the screen instead.Open a side panel and it gets worse. Happen to be watching at the host's aspect ratio and it looks perfect. That combination is why this presents as flaky rather than broken, and why it survived this long.
The fix
The geometry already existed and was already correct.
getContainRectwas written for the camera bubble, which had to be positioned against the picture for exactly this reason — it just was never used by the input path. It moves to@pairux/shared-typesso there is one implementation, since an overlay drawn against one rectangle and clicks mapped against another is its own bug.Three places were reading the wrong rectangle:
Positions inside the letterbox bars clamp to the nearest edge of the picture rather than being dropped, for the same reason coordinates are clamped everywhere else — the screen edges have to stay reachable.
Notes
useVideoContentRectre-measures on resize, onloadedmetadata, and on the video'sresizeevent, sincevideoWidthis 0 until the stream reports it and the shape changes when the host switches which screen they share.getContainRectfalls back to the full box — identical to the old behaviour, so there is no window where this is worse than before.ResizeObserverstand-in; jsdom does not implement it.lint0 errors,typecheck9/9,build7/7. 1443 tests pass, +8 covering the mapping — including the menu-bar case above stated as a regression test.Still not run on real hardware, same as #59.
🤖 Generated with Claude Code