diff --git a/apps/desktop/package.json b/apps/desktop/package.json index e1209db..0764902 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -1,6 +1,6 @@ { "name": "@pairux/desktop", - "version": "0.9.54", + "version": "0.9.55", "private": true, "description": "PairUX Desktop - Screen sharing with remote control", "author": "PairUX Team ", diff --git a/apps/desktop/src/main/input/injector.test.ts b/apps/desktop/src/main/input/injector.test.ts index 8854679..b9c8b09 100644 --- a/apps/desktop/src/main/input/injector.test.ts +++ b/apps/desktop/src/main/input/injector.test.ts @@ -301,12 +301,14 @@ describe('Input Injector', () => { }); describe('mouse scroll events', () => { - it('should handle scroll down event', async () => { + // DOM WheelEvent: positive deltaY is a scroll *down*. Both backends had + // this backwards, so every remote scroll went the wrong way. + it('scrolls down for positive deltaY', async () => { const event: MouseScrollEvent = { type: 'mouse', action: 'scroll', deltaX: 0, - deltaY: -120, + deltaY: 120, x: 0.5, y: 0.5, }; @@ -315,14 +317,15 @@ describe('Input Injector', () => { expect(mouse.setPosition).toHaveBeenCalled(); expect(mouse.scrollDown).toHaveBeenCalled(); + expect(mouse.scrollUp).not.toHaveBeenCalled(); }); - it('should handle scroll up event', async () => { + it('scrolls up for negative deltaY', async () => { const event: MouseScrollEvent = { type: 'mouse', action: 'scroll', deltaX: 0, - deltaY: 120, + deltaY: -120, x: 0.5, y: 0.5, }; diff --git a/apps/desktop/src/renderer/components/capture/CapturePreview.tsx b/apps/desktop/src/renderer/components/capture/CapturePreview.tsx index e784d2f..c5eb4b9 100644 --- a/apps/desktop/src/renderer/components/capture/CapturePreview.tsx +++ b/apps/desktop/src/renderer/components/capture/CapturePreview.tsx @@ -344,20 +344,12 @@ export function CapturePreview({ }; }, [hasPendingControlRequests]); - const inputScreenSize = useMemo(() => { - if (!stream) return undefined; - const tracks = stream.getVideoTracks(); - const track = tracks.length > 0 ? tracks[0] : undefined; - const settings = track?.getSettings(); - const width = settings?.width; - const height = settings?.height; - if (!width || !height) return undefined; - return { width, height }; - }, [stream]); - + // No screenSize is passed on purpose: the injection backend reads the host's + // real screen geometry from the OS itself. The capture track's dimensions are + // the *encoded stream* size, which is a different number in different units, + // and using it put every remote click in the wrong place. See useInputInjection. const { injectEvent, diagnostics: inputDiagnostics } = useInputInjection({ enabled: Boolean(participantWithControl) || grantedViewerId !== null, - screenSize: inputScreenSize, }); const remoteInputCountRef = useRef(0); const showWaylandInputDiagnostics = diff --git a/apps/desktop/src/renderer/hooks/useInputInjection.test.ts b/apps/desktop/src/renderer/hooks/useInputInjection.test.ts index 66a4e99..12850d4 100644 --- a/apps/desktop/src/renderer/hooks/useInputInjection.test.ts +++ b/apps/desktop/src/renderer/hooks/useInputInjection.test.ts @@ -182,32 +182,22 @@ describe('useInputInjection', () => { }); }); - describe('screen size updates', () => { - it('should update screen size when screenSize prop changes', async () => { - const { rerender } = renderHook( - ({ screenSize }) => useInputInjection({ enabled: false, screenSize }), - { initialProps: { screenSize: { width: 1920, height: 1080 } } } - ); + // Regression: the hook used to forward the capture track's dimensions as the + // injection screen size. Those are encoded-stream pixels, not the host's + // screen geometry, and on a Retina Mac (logical 1440x900, stream 2880x1800) + // that mapped the centre of the screen to its bottom-right corner and put + // everything past halfway off the display, so clicks did nothing. The backend + // reads its own geometry from the OS; the renderer must not override it. + describe('screen size', () => { + it('never sends a screen size to the backend', async () => { + renderHook(() => useInputInjection({ enabled: true })); await act(async () => { await vi.runAllTimersAsync(); }); - expect(mockElectronAPI.invoke).toHaveBeenCalledWith('input:updateScreenSize', { - width: 1920, - height: 1080, - }); - - rerender({ screenSize: { width: 2560, height: 1440 } }); - - await act(async () => { - await vi.runAllTimersAsync(); - }); - - expect(mockElectronAPI.invoke).toHaveBeenCalledWith('input:updateScreenSize', { - width: 2560, - height: 1440, - }); + const channels = mockElectronAPI.invoke.mock.calls.map(([channel]) => channel); + expect(channels).not.toContain('input:updateScreenSize'); }); }); diff --git a/apps/desktop/src/renderer/hooks/useInputInjection.ts b/apps/desktop/src/renderer/hooks/useInputInjection.ts index 8f8ba58..d8cab51 100644 --- a/apps/desktop/src/renderer/hooks/useInputInjection.ts +++ b/apps/desktop/src/renderer/hooks/useInputInjection.ts @@ -10,8 +10,6 @@ import type { InputInjectionDiagnostics } from '../../preload/api'; interface UseInputInjectionOptions { /** Whether injection should be active */ enabled: boolean; - /** Screen dimensions for coordinate mapping */ - screenSize?: { width: number; height: number }; /** Callback when emergency stop is triggered */ onEmergencyStop?: () => void; } @@ -36,7 +34,6 @@ interface UseInputInjectionReturn { */ export function useInputInjection({ enabled, - screenSize, onEmergencyStop, }: UseInputInjectionOptions): UseInputInjectionReturn { const [isEnabled, setIsEnabled] = useState(false); @@ -86,21 +83,17 @@ export function useInputInjection({ void init(); }, []); - // Update screen size when it changes - useEffect(() => { - if (!isInitialized || !screenSize) return; - - const updateSize = async () => { - try { - await window.electronAPI.invoke('input:updateScreenSize', screenSize); - } catch (error) { - console.error('[useInputInjection] Failed to update screen size:', error); - } - }; - - void updateSize(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isInitialized, screenSize?.width, screenSize?.height]); + // Deliberately does not tell the backend a screen size. + // + // Remote coordinates are normalized 0-1, so the only thing needed to place + // them is the host's own screen geometry — which each backend already reads + // from its own OS API in that API's units (nut-js from screen.width(), the + // Wayland backend from the compositor). Feeding it anything else mixes units. + // + // This used to pass the capture track's dimensions, which are neither: the + // track reports encoded pixels, so a Retina Mac (logical 1440x900, stream + // 2880x1800) mapped the centre of the screen to its bottom-right corner and + // everything past halfway fell off the display entirely. // Enable/disable injection based on prop useEffect(() => { diff --git a/apps/desktop/src/renderer/hooks/useRemoteControl.ts b/apps/desktop/src/renderer/hooks/useRemoteControl.ts index 51b3a02..e8477e6 100644 --- a/apps/desktop/src/renderer/hooks/useRemoteControl.ts +++ b/apps/desktop/src/renderer/hooks/useRemoteControl.ts @@ -1,4 +1,4 @@ -import { useCallback, useRef, useEffect, useState } from 'react'; +import { useCallback, useMemo, useRef, useEffect, useState } from 'react'; import type { InputEvent, MouseMoveEvent, @@ -8,6 +8,8 @@ import type { MouseButton, ControlStateUI, } from '@pairux/shared-types'; +import { modifiersFromDomEvent } from '@pairux/shared-types'; +import { getAccelPlatform } from '@/lib/viewerPlatform'; interface UseRemoteControlOptions { enabled: boolean; @@ -45,6 +47,9 @@ export function useRemoteControl({ onCursorMove, }: UseRemoteControlOptions): UseRemoteControlReturn { const [isCapturing, setIsCapturing] = useState(false); + // Read once: it cannot change while the app runs. Kept out of module scope so + // importing this hook has no side effects. + const viewerPlatform = useMemo(() => getAccelPlatform(), []); // Buttons/keys this viewer has sent a "down" for. Every one of them must get // an "up", or the host is left mid-drag with a stuck button. const heldButtonsRef = useRef>(new Set()); @@ -228,19 +233,14 @@ export function useRemoteControl({ action: 'down', key: event.key, code: event.code, - modifiers: { - ctrl: event.ctrlKey, - alt: event.altKey, - shift: event.shiftKey, - meta: event.metaKey, - }, + modifiers: modifiersFromDomEvent(event, viewerPlatform), }; heldKeysRef.current.add(inputEvent.code); onInputEvent(inputEvent); event.preventDefault(); }, - [canSendInput, onInputEvent] + [canSendInput, onInputEvent, viewerPlatform] ); // Handle key up @@ -253,18 +253,13 @@ export function useRemoteControl({ action: 'up', key: event.key, code: event.code, - modifiers: { - ctrl: event.ctrlKey, - alt: event.altKey, - shift: event.shiftKey, - meta: event.metaKey, - }, + modifiers: modifiersFromDomEvent(event, viewerPlatform), }; heldKeysRef.current.delete(inputEvent.code); onInputEvent(inputEvent); }, - [canSendInput, onInputEvent] + [canSendInput, onInputEvent, viewerPlatform] ); // Handle context menu (right-click) diff --git a/apps/desktop/src/renderer/lib/viewerPlatform.test.ts b/apps/desktop/src/renderer/lib/viewerPlatform.test.ts new file mode 100644 index 0000000..2106e3b --- /dev/null +++ b/apps/desktop/src/renderer/lib/viewerPlatform.test.ts @@ -0,0 +1,34 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { accelPlatformFor, getAccelPlatform } from './viewerPlatform'; + +describe('accelPlatformFor', () => { + it('treats darwin as the Cmd platform', () => { + expect(accelPlatformFor('darwin')).toBe('darwin'); + }); + + it('treats everything else as the Ctrl platform', () => { + expect(accelPlatformFor('linux')).toBe('other'); + expect(accelPlatformFor('win32')).toBe('other'); + }); + + // Never throws on render: a wrong modifier beats a blank screen. + it('falls back to the Ctrl platform when the platform is unknown', () => { + expect(accelPlatformFor(undefined)).toBe('other'); + expect(accelPlatformFor(null)).toBe('other'); + }); +}); + +describe('getAccelPlatform', () => { + afterEach(() => { + delete (globalThis as { electronAPI?: unknown }).electronAPI; + }); + + it('reads the platform exposed by preload', () => { + (globalThis as { electronAPI?: unknown }).electronAPI = { platform: 'darwin' }; + expect(getAccelPlatform()).toBe('darwin'); + }); + + it('does not throw when preload is absent', () => { + expect(getAccelPlatform()).toBe('other'); + }); +}); diff --git a/apps/desktop/src/renderer/lib/viewerPlatform.ts b/apps/desktop/src/renderer/lib/viewerPlatform.ts new file mode 100644 index 0000000..046602c --- /dev/null +++ b/apps/desktop/src/renderer/lib/viewerPlatform.ts @@ -0,0 +1,26 @@ +/** + * Which modifier this machine uses for keyboard shortcuts. + * + * Needed when acting as a viewer, because the host may be on a different OS: + * "the shortcut key" is Cmd on macOS and Ctrl everywhere else, and only the + * viewer knows which one the user actually pressed. See `modifiersFromDomEvent`. + */ + +export type AccelPlatform = 'darwin' | 'other'; + +/** Pure so the mapping is testable without a window. */ +export function accelPlatformFor(platform: string | undefined | null): AccelPlatform { + return platform === 'darwin' ? 'darwin' : 'other'; +} + +/** + * Read the platform from preload rather than sniffing the user agent. + * + * Falls back to 'other' outside Electron, which is also the right answer for a + * plain browser on anything but a Mac — and a wrong shortcut modifier is a + * better failure than throwing on render. + */ +export function getAccelPlatform(): AccelPlatform { + const api = (globalThis as { electronAPI?: { platform?: string } }).electronAPI; + return accelPlatformFor(api?.platform); +} diff --git a/apps/installer/package.json b/apps/installer/package.json index 96b0423..f420f74 100644 --- a/apps/installer/package.json +++ b/apps/installer/package.json @@ -1,6 +1,6 @@ { "name": "@pairux/installer", - "version": "0.9.54", + "version": "0.9.55", "private": true, "description": "PairUX Desktop App Installer Service", "type": "module", diff --git a/apps/livekit/package.json b/apps/livekit/package.json index b032dc1..4a8ea80 100644 --- a/apps/livekit/package.json +++ b/apps/livekit/package.json @@ -1,6 +1,6 @@ { "name": "@pairux/livekit", - "version": "0.9.54", + "version": "0.9.55", "private": true, "description": "PairUX LiveKit SFU Server", "scripts": { diff --git a/apps/turn/package.json b/apps/turn/package.json index 4e09184..83441ee 100644 --- a/apps/turn/package.json +++ b/apps/turn/package.json @@ -1,6 +1,6 @@ { "name": "@pairux/turn", - "version": "0.9.54", + "version": "0.9.55", "private": true, "description": "PairUX TURN/STUN Server (coturn)", "scripts": { diff --git a/apps/web/package.json b/apps/web/package.json index b6c04e4..479fbb9 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -1,6 +1,6 @@ { "name": "@pairux/web", - "version": "0.9.54", + "version": "0.9.55", "private": true, "type": "module", "scripts": { diff --git a/apps/web/src/hooks/useRemoteControl.ts b/apps/web/src/hooks/useRemoteControl.ts index 51b3a02..e8477e6 100644 --- a/apps/web/src/hooks/useRemoteControl.ts +++ b/apps/web/src/hooks/useRemoteControl.ts @@ -1,4 +1,4 @@ -import { useCallback, useRef, useEffect, useState } from 'react'; +import { useCallback, useMemo, useRef, useEffect, useState } from 'react'; import type { InputEvent, MouseMoveEvent, @@ -8,6 +8,8 @@ import type { MouseButton, ControlStateUI, } from '@pairux/shared-types'; +import { modifiersFromDomEvent } from '@pairux/shared-types'; +import { getAccelPlatform } from '@/lib/viewerPlatform'; interface UseRemoteControlOptions { enabled: boolean; @@ -45,6 +47,9 @@ export function useRemoteControl({ onCursorMove, }: UseRemoteControlOptions): UseRemoteControlReturn { const [isCapturing, setIsCapturing] = useState(false); + // Read once: it cannot change while the app runs. Kept out of module scope so + // importing this hook has no side effects. + const viewerPlatform = useMemo(() => getAccelPlatform(), []); // Buttons/keys this viewer has sent a "down" for. Every one of them must get // an "up", or the host is left mid-drag with a stuck button. const heldButtonsRef = useRef>(new Set()); @@ -228,19 +233,14 @@ export function useRemoteControl({ action: 'down', key: event.key, code: event.code, - modifiers: { - ctrl: event.ctrlKey, - alt: event.altKey, - shift: event.shiftKey, - meta: event.metaKey, - }, + modifiers: modifiersFromDomEvent(event, viewerPlatform), }; heldKeysRef.current.add(inputEvent.code); onInputEvent(inputEvent); event.preventDefault(); }, - [canSendInput, onInputEvent] + [canSendInput, onInputEvent, viewerPlatform] ); // Handle key up @@ -253,18 +253,13 @@ export function useRemoteControl({ action: 'up', key: event.key, code: event.code, - modifiers: { - ctrl: event.ctrlKey, - alt: event.altKey, - shift: event.shiftKey, - meta: event.metaKey, - }, + modifiers: modifiersFromDomEvent(event, viewerPlatform), }; heldKeysRef.current.delete(inputEvent.code); onInputEvent(inputEvent); }, - [canSendInput, onInputEvent] + [canSendInput, onInputEvent, viewerPlatform] ); // Handle context menu (right-click) diff --git a/apps/web/src/lib/viewerPlatform.test.ts b/apps/web/src/lib/viewerPlatform.test.ts new file mode 100644 index 0000000..59b60e6 --- /dev/null +++ b/apps/web/src/lib/viewerPlatform.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest'; +import { accelPlatformFor } from './viewerPlatform'; + +describe('accelPlatformFor', () => { + it('detects a Mac from userAgentData', () => { + expect(accelPlatformFor('macOS')).toBe('darwin'); + }); + + it('detects a Mac from a user agent string', () => { + expect( + accelPlatformFor( + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko)' + ) + ).toBe('darwin'); + }); + + it('treats Linux and Windows as the Ctrl platform', () => { + expect(accelPlatformFor('Linux x86_64')).toBe('other'); + expect(accelPlatformFor('Mozilla/5.0 (X11; Linux x86_64)')).toBe('other'); + expect(accelPlatformFor('Windows')).toBe('other'); + expect(accelPlatformFor('Mozilla/5.0 (Windows NT 10.0; Win64; x64)')).toBe('other'); + }); + + // Never throws during render: a wrong modifier beats a blank screen. + it('falls back to the Ctrl platform without a hint', () => { + expect(accelPlatformFor(undefined)).toBe('other'); + expect(accelPlatformFor(null)).toBe('other'); + expect(accelPlatformFor('')).toBe('other'); + }); +}); diff --git a/apps/web/src/lib/viewerPlatform.ts b/apps/web/src/lib/viewerPlatform.ts new file mode 100644 index 0000000..3d17266 --- /dev/null +++ b/apps/web/src/lib/viewerPlatform.ts @@ -0,0 +1,29 @@ +/** + * Which modifier this machine uses for keyboard shortcuts. + * + * Needed when acting as a viewer, because the host may be on a different OS: + * "the shortcut key" is Cmd on macOS and Ctrl everywhere else, and only the + * viewer knows which one the user actually pressed. See `modifiersFromDomEvent`. + */ + +export type AccelPlatform = 'darwin' | 'other'; + +/** + * Pure so the mapping is testable without a browser. + * + * `navigator.platform` is deprecated and `userAgentData` is Chromium-only, so + * whichever string the caller could get is matched loosely for "mac". + */ +export function accelPlatformFor(platformHint: string | undefined | null): AccelPlatform { + return platformHint && /mac/i.test(platformHint) ? 'darwin' : 'other'; +} + +export function getAccelPlatform(): AccelPlatform { + if (typeof navigator === 'undefined') return 'other'; + + const hint = + (navigator as Navigator & { userAgentData?: { platform?: string } }).userAgentData?.platform ?? + navigator.userAgent; + + return accelPlatformFor(hint); +} diff --git a/package.json b/package.json index e034f1b..47c6721 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "pairux", - "version": "0.9.54", + "version": "0.9.55", "private": true, "description": "Collaborative desktop screen sharing with remote control", "type": "module", diff --git a/packages/remote-input/src/backends/nutjs.ts b/packages/remote-input/src/backends/nutjs.ts index 6050c33..bafabf3 100644 --- a/packages/remote-input/src/backends/nutjs.ts +++ b/packages/remote-input/src/backends/nutjs.ts @@ -1,3 +1,4 @@ +import { resolveModifiers } from '../modifiers.js'; import type { InputEvent, MouseMoveEvent, @@ -169,7 +170,8 @@ export class NutJsInputBackend implements InputBackend { if (event.deltaY !== 0) { const scrollAmount = Math.abs(Math.round(event.deltaY / 100)) || 1; - if (event.deltaY < 0) await mouse.scrollDown(scrollAmount); + // DOM deltaY is positive when scrolling down. + if (event.deltaY > 0) await mouse.scrollDown(scrollAmount); else await mouse.scrollUp(scrollAmount); } @@ -185,11 +187,14 @@ export class NutJsInputBackend implements InputBackend { const key = mapKey(event.key, event.code, Key); const { modifiers } = event; + // Resolved against *this* host, so a Mac viewer's Cmd+C becomes Ctrl+C here + // rather than Super+C, and vice versa. LeftSuper is Cmd on macOS. + const resolved = resolveModifiers(modifiers, process.platform); const modifierKeys: NutModule['Key'][keyof NutModule['Key']][] = []; - if (modifiers.ctrl) modifierKeys.push(Key.LeftControl); - if (modifiers.alt) modifierKeys.push(Key.LeftAlt); - if (modifiers.shift) modifierKeys.push(Key.LeftShift); - if (modifiers.meta) modifierKeys.push(Key.LeftSuper); + if (resolved.control) modifierKeys.push(Key.LeftControl); + if (resolved.alt) modifierKeys.push(Key.LeftAlt); + if (resolved.shift) modifierKeys.push(Key.LeftShift); + if (resolved.meta) modifierKeys.push(Key.LeftSuper); switch (event.action) { case 'down': diff --git a/packages/remote-input/src/backends/waylandYdotool.test.ts b/packages/remote-input/src/backends/waylandYdotool.test.ts index 09f0505..b60c842 100644 --- a/packages/remote-input/src/backends/waylandYdotool.test.ts +++ b/packages/remote-input/src/backends/waylandYdotool.test.ts @@ -164,7 +164,32 @@ describe('WaylandYdotoolInputBackend', () => { await backend.inject(event); expect(run).toHaveBeenNthCalledWith(1, 'ydotool', ['mousemove', '--absolute', '400', '600']); - expect(run).toHaveBeenNthCalledWith(2, 'ydotool', ['click', '--repeat', '2', '196']); + // Negative deltaY is a scroll *up* => button 4, base 0x03 => 0xC3 = 195. + expect(run).toHaveBeenNthCalledWith(2, 'ydotool', ['click', '--repeat', '2', '195']); + }); + + // Regression: both backends treated positive deltaY as "up", so every remote + // scroll went the wrong way. DOM deltaY is positive when scrolling down. + it('scrolls down for positive deltaY', async () => { + const run = vi.fn().mockResolvedValue(undefined); + const backend = new WaylandYdotoolInputBackend(run, { + hasBinary: true, + hasSocket: true, + socketPath: '/tmp/.ydotool_socket', + }); + backend.updateScreenSize(1000, 1000); + + await backend.inject({ + type: 'mouse', + action: 'scroll', + deltaX: 0, + deltaY: 120, + x: 0.4, + y: 0.6, + }); + + // button 5 (down), base 0x04 => 0xC4 = 196. + expect(run).toHaveBeenNthCalledWith(2, 'ydotool', ['click', '196']); }); it('emits horizontal scroll via wheel click codes', async () => { diff --git a/packages/remote-input/src/backends/waylandYdotool.ts b/packages/remote-input/src/backends/waylandYdotool.ts index f2e14f4..8c516cd 100644 --- a/packages/remote-input/src/backends/waylandYdotool.ts +++ b/packages/remote-input/src/backends/waylandYdotool.ts @@ -2,6 +2,7 @@ import { execFileSync } from 'child_process'; import { existsSync } from 'fs'; import { KWinCursorProvider } from '../wayland/kwinCursorProvider.js'; import { detectWaylandScreenSize, type ScreenSize } from '../wayland/screenSize.js'; +import { resolveModifiers } from '../modifiers.js'; import type { InputEvent, MouseMoveEvent, @@ -317,11 +318,14 @@ function keyCodeFromEvent(event: KbEvent): number | null { } function modifierKeycodes(modifiers: KbEvent['modifiers']): number[] { + // This host is Linux, so the shortcut modifier is Control: a Mac viewer's + // Cmd+C has to land as Ctrl+C, not as Super+C. + const resolved = resolveModifiers(modifiers, 'linux'); const keys: number[] = []; - if (modifiers.ctrl) keys.push(KEYCODES.ControlLeft); - if (modifiers.alt) keys.push(KEYCODES.AltLeft); - if (modifiers.shift) keys.push(KEYCODES.ShiftLeft); - if (modifiers.meta) keys.push(KEYCODES.MetaLeft); + if (resolved.control) keys.push(KEYCODES.ControlLeft); + if (resolved.alt) keys.push(KEYCODES.AltLeft); + if (resolved.shift) keys.push(KEYCODES.ShiftLeft); + if (resolved.meta) keys.push(KEYCODES.MetaLeft); return keys; } @@ -526,7 +530,8 @@ export class WaylandYdotoolInputBackend implements InputBackend { // button 4 (up), 5 (down), 6 (left), 7 (right) => bases 3,4,5,6. if (event.deltaY !== 0) { const repeat = scrollRepeat(event.deltaY); - const base = event.deltaY > 0 ? 0x03 : 0x04; + // DOM deltaY is positive when scrolling down, which is button 5 (base 4). + const base = event.deltaY > 0 ? 0x04 : 0x03; await this.run(this.ydotoolCommand, ['click', ...scrollClickCode(base, repeat)]); } diff --git a/packages/remote-input/src/modifiers.test.ts b/packages/remote-input/src/modifiers.test.ts new file mode 100644 index 0000000..94ea5c7 --- /dev/null +++ b/packages/remote-input/src/modifiers.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it } from 'vitest'; +import { resolveModifiers } from './modifiers.js'; +import type { KeyboardModifiers } from './types.js'; + +const none: KeyboardModifiers = { ctrl: false, alt: false, shift: false, meta: false }; + +describe('resolveModifiers', () => { + // The two cases that were broken in both directions. + it('turns a Mac viewer\u2019s Cmd into Control on a Linux host', () => { + expect(resolveModifiers({ ...none, accel: true }, 'linux')).toEqual({ + control: true, + alt: false, + shift: false, + meta: false, + }); + }); + + it('turns a Linux viewer\u2019s Ctrl into Cmd on a macOS host', () => { + expect(resolveModifiers({ ...none, accel: true }, 'darwin')).toEqual({ + control: false, + alt: false, + shift: false, + meta: true, + }); + }); + + it('maps the accelerator to Control on Windows', () => { + expect(resolveModifiers({ ...none, accel: true }, 'win32')).toMatchObject({ + control: true, + meta: false, + }); + }); + + it('keeps a literal Control literal on a macOS host', () => { + expect(resolveModifiers({ ...none, ctrl: true }, 'darwin')).toMatchObject({ + control: true, + meta: false, + }); + }); + + it('keeps a literal Super literal on a Linux host', () => { + expect(resolveModifiers({ ...none, meta: true }, 'linux')).toMatchObject({ + control: false, + meta: true, + }); + }); + + // Control+Cmd+F (fullscreen on macOS) has to arrive intact. + it('combines a literal Control with the accelerator on macOS', () => { + expect(resolveModifiers({ ...none, ctrl: true, accel: true }, 'darwin')).toMatchObject({ + control: true, + meta: true, + }); + }); + + it('does not double up when the accelerator is already Control', () => { + expect(resolveModifiers({ ...none, ctrl: true, accel: true }, 'linux')).toMatchObject({ + control: true, + meta: false, + }); + }); + + // An older viewer sends no `accel` at all; nothing should be invented. + it('falls back to literal modifiers when accel is absent', () => { + expect(resolveModifiers({ ...none, ctrl: true }, 'linux')).toEqual({ + control: true, + alt: false, + shift: false, + meta: false, + }); + expect(resolveModifiers(none, 'darwin')).toEqual({ + control: false, + alt: false, + shift: false, + meta: false, + }); + }); +}); diff --git a/packages/remote-input/src/modifiers.ts b/packages/remote-input/src/modifiers.ts new file mode 100644 index 0000000..cf8ad15 --- /dev/null +++ b/packages/remote-input/src/modifiers.ts @@ -0,0 +1,46 @@ +/** + * Which modifiers a backend should actually hold down. + * + * The wire carries the viewer's modifiers as the viewer's OS reported them, + * which is not portable: "the shortcut key" is Cmd on macOS and Ctrl elsewhere, + * and injecting the literal one on a different OS produces a keystroke nobody + * asked for. Cmd+C arriving on Linux as Super+C copies nothing; Ctrl+C arriving + * on macOS as Control+C copies nothing either. + * + * So `accel` says "the shortcut modifier was held" and this decides what that + * means locally, leaving `ctrl`/`meta` for the cases that really do mean + * Control (macOS Control+click) or Super (Linux window manager bindings). + */ + +import type { KeyboardModifiers, Platform } from './types.js'; + +/** Modifiers resolved for a specific host, with no platform ambiguity left. */ +export interface ResolvedModifiers { + control: boolean; + alt: boolean; + shift: boolean; + /** Cmd on macOS, Super/Win elsewhere. */ + meta: boolean; +} + +export function resolveModifiers( + modifiers: KeyboardModifiers, + platform: Platform +): ResolvedModifiers { + const hostUsesMetaForShortcuts = platform === 'darwin'; + const accel = modifiers.accel === true; + + return { + // On a non-macOS host the shortcut modifier *is* Control, so fold accel in. + control: modifiers.ctrl || (accel && !hostUsesMetaForShortcuts), + alt: modifiers.alt, + shift: modifiers.shift, + // On macOS the shortcut modifier is Cmd, which is the meta key. + meta: modifiers.meta || (accel && hostUsesMetaForShortcuts), + }; +} + +// The viewer half of this — turning a DOM event into wire modifiers — lives in +// @pairux/shared-types as `modifiersFromDomEvent`, because this package is +// deliberately standalone and Node-only while the viewers run in a browser. +// The two must stay in step: `accel` set there is what `resolveModifiers` reads. diff --git a/packages/remote-input/src/types.ts b/packages/remote-input/src/types.ts index 8f78d54..d77081b 100644 --- a/packages/remote-input/src/types.ts +++ b/packages/remote-input/src/types.ts @@ -42,6 +42,15 @@ export interface KeyboardModifiers { shift: boolean; /** Cmd on macOS, Win on Windows, Super on Linux. */ meta: boolean; + /** + * The viewer held their platform's shortcut modifier: Cmd on macOS, Ctrl + * everywhere else. Backends map it to whichever modifier means "shortcut" on + * *this* host, so a shortcut survives crossing operating systems — a Mac + * viewer's Cmd+C has to become Ctrl+C on a Linux host, not Super+C. + * + * Optional: an older viewer that omits it keeps the literal pass-through. + */ + accel?: boolean; } export interface KeyboardInputEvent { diff --git a/packages/shared-types/package.json b/packages/shared-types/package.json index 2356cb9..d57f660 100644 --- a/packages/shared-types/package.json +++ b/packages/shared-types/package.json @@ -1,6 +1,6 @@ { "name": "@pairux/shared-types", - "version": "0.9.54", + "version": "0.9.55", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/shared-types/src/index.ts b/packages/shared-types/src/index.ts index caa0f95..09bd2ed 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -100,7 +100,7 @@ export type { QualityPreset, } from './input.js'; -export { QUALITY_PRESETS } from './input.js'; +export { QUALITY_PRESETS, modifiersFromDomEvent } from './input.js'; // Voice audio settings shared by every client export { diff --git a/packages/shared-types/src/input.test.ts b/packages/shared-types/src/input.test.ts index c4360eb..183d815 100644 --- a/packages/shared-types/src/input.test.ts +++ b/packages/shared-types/src/input.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from 'vitest'; import { QUALITY_PRESETS, + modifiersFromDomEvent, type MouseEvent, type KeyboardEvent, type InputMessage, @@ -198,4 +199,69 @@ describe('Input Types', () => { }); }); }); + + // A shortcut has to survive crossing operating systems. "The shortcut key" is + // Cmd on macOS and Ctrl elsewhere, so sending the literal flag meant a Mac + // viewer's Cmd+C reached a Linux host as Super+C, and a Linux viewer's Ctrl+C + // reached a macOS host as Control+C. Neither copies anything. + describe('modifiersFromDomEvent', () => { + type DomModifierFlags = Record<'ctrlKey' | 'altKey' | 'shiftKey' | 'metaKey', boolean>; + + const dom = (held: Partial): DomModifierFlags => ({ + ctrlKey: held.ctrlKey ?? false, + altKey: held.altKey ?? false, + shiftKey: held.shiftKey ?? false, + metaKey: held.metaKey ?? false, + }); + + it('reports a Mac viewer\u2019s Cmd as the accelerator, not as meta', () => { + expect(modifiersFromDomEvent(dom({ metaKey: true }), 'darwin')).toEqual({ + ctrl: false, + alt: false, + shift: false, + meta: false, + accel: true, + }); + }); + + it('reports Ctrl elsewhere as the accelerator, not as ctrl', () => { + expect(modifiersFromDomEvent(dom({ ctrlKey: true }), 'other')).toEqual({ + ctrl: false, + alt: false, + shift: false, + meta: false, + accel: true, + }); + }); + + // On macOS, Control is its own modifier (Control+click is a right click). + it('keeps macOS Control literal', () => { + expect(modifiersFromDomEvent(dom({ ctrlKey: true }), 'darwin')).toMatchObject({ + ctrl: true, + accel: false, + }); + }); + + it('keeps Super literal for non-macOS viewers', () => { + expect(modifiersFromDomEvent(dom({ metaKey: true }), 'other')).toMatchObject({ + meta: true, + accel: false, + }); + }); + + it('passes alt and shift straight through', () => { + expect(modifiersFromDomEvent(dom({ altKey: true, shiftKey: true }), 'other')).toMatchObject({ + alt: true, + shift: true, + accel: false, + }); + }); + + it('carries both when macOS Control accompanies Cmd', () => { + expect(modifiersFromDomEvent(dom({ ctrlKey: true, metaKey: true }), 'darwin')).toMatchObject({ + ctrl: true, + accel: true, + }); + }); + }); }); diff --git a/packages/shared-types/src/input.ts b/packages/shared-types/src/input.ts index 90696a9..7cb9d31 100644 --- a/packages/shared-types/src/input.ts +++ b/packages/shared-types/src/input.ts @@ -26,7 +26,14 @@ export interface MouseButtonEvent { export interface MouseScrollEvent { type: 'mouse'; action: 'scroll'; + /** DOM WheelEvent convention: positive scrolls right. */ deltaX: number; + /** + * DOM WheelEvent convention: **positive scrolls down**. + * + * Spelled out because both host backends had it backwards, which made every + * remote scroll go the wrong way on every platform. + */ deltaY: number; x: number; y: number; @@ -41,6 +48,56 @@ export interface KeyboardModifiers { alt: boolean; shift: boolean; meta: boolean; // Cmd on macOS, Win on Windows + /** + * The viewer held their platform's shortcut modifier: Cmd on macOS, Ctrl + * everywhere else. + * + * Sent instead of the literal modifier, because the literal one does not + * survive crossing operating systems. A Mac viewer pressing Cmd+C reports + * `meta`, which on a Linux host is Super — and Super+C copies nothing. + * A Linux viewer pressing Ctrl+C reports `ctrl`, which on a macOS host is + * Control, and Control+C is not copy either. So copy/paste, save, quit and + * every other shortcut broke in both directions. + * + * The host maps this to whichever modifier means "shortcut" locally. `ctrl` + * and `meta` stay literal for the cases that genuinely mean Control or Super + * (macOS Control+click, Linux Super for the window manager). + * + * Optional for compatibility: an older viewer that omits it still gets the + * previous literal pass-through behaviour. + */ + accel?: boolean; +} + +/** + * Turn a DOM keyboard event's modifier flags into portable wire modifiers. + * + * Viewers must use this rather than copying `ctrlKey`/`metaKey` straight across. + * "The shortcut key" is Cmd on macOS and Ctrl everywhere else, so the literal + * flag does not survive a change of operating system: Cmd+C sent as `meta` + * arrives on Linux as Super+C, and Ctrl+C sent as `ctrl` arrives on macOS as + * Control+C. Neither copies anything. + * + * Whichever key acted as the accelerator is reported as `accel` alone, so the + * host presses one shortcut modifier instead of its own plus the viewer's. The + * host side of this is `resolveModifiers` in @profullstack/remote-input. + */ +export function modifiersFromDomEvent( + event: { ctrlKey: boolean; altKey: boolean; shiftKey: boolean; metaKey: boolean }, + platform: 'darwin' | 'other' +): KeyboardModifiers { + const isMac = platform === 'darwin'; + + return { + // macOS Control is a real modifier of its own (Control+click), so it stays + // literal there. Off macOS, Ctrl is the accelerator and nothing else. + ctrl: isMac ? event.ctrlKey : false, + alt: event.altKey, + shift: event.shiftKey, + // Super/Win is literal off macOS; on macOS Cmd is the accelerator. + meta: isMac ? false : event.metaKey, + accel: isMac ? event.metaKey : event.ctrlKey, + }; } // Keyboard event