From 77599b1f037c3bb6a5edf45fef5e13fbfd4d4c88 Mon Sep 17 00:00:00 2001 From: Dan Share Date: Wed, 13 Aug 2025 14:46:38 +0100 Subject: [PATCH] Track and sync modifier states --- .../DesktopSession/DesktopSession.tsx | 2 + .../DesktopSession/InputHandler.test.ts | 192 ++++++++++++++++++ .../DesktopSession/InputHandler.tsx | 90 +++++++- 3 files changed, 283 insertions(+), 1 deletion(-) create mode 100644 web/packages/shared/components/DesktopSession/InputHandler.test.ts diff --git a/web/packages/shared/components/DesktopSession/DesktopSession.tsx b/web/packages/shared/components/DesktopSession/DesktopSession.tsx index 5199f9b88a42e..903061b7151e3 100644 --- a/web/packages/shared/components/DesktopSession/DesktopSession.tsx +++ b/web/packages/shared/components/DesktopSession/DesktopSession.tsx @@ -303,6 +303,8 @@ export function DesktopSession({ function handleMouseWheel(e: WheelEvent) { e.preventDefault(); + // Check modifier states are correct to avoid unintentional zooming whilst scrolling. + inputHandler.current.synchronizeModifierState(client, e); // We only support pixel scroll events, not line or page events. // https://developer.mozilla.org/en-US/docs/Web/API/WheelEvent/deltaMode if (e.deltaMode === WheelEvent.DOM_DELTA_PIXEL) { diff --git a/web/packages/shared/components/DesktopSession/InputHandler.test.ts b/web/packages/shared/components/DesktopSession/InputHandler.test.ts new file mode 100644 index 0000000000000..7345e2da1f056 --- /dev/null +++ b/web/packages/shared/components/DesktopSession/InputHandler.test.ts @@ -0,0 +1,192 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { + ButtonState, + selectDirectoryInBrowser, + TdpClient, +} from 'shared/libs/tdp'; + +import { InputHandler } from './InputHandler'; + +// Mock the TdpClient class +jest.mock('shared/libs/tdp', () => { + const originalModule = jest.requireActual('shared/libs/tdp'); + return { + ...originalModule, + TdpClient: jest.fn().mockImplementation(() => { + return { + sendKeyboardInput: jest.fn(), + sendMouseButton: jest.fn(), + sendSyncKeys: jest.fn(), + }; + }), + }; +}); + +describe('InputHandler', () => { + let inputHandler: InputHandler; + let mockTdpClient: jest.Mocked; + + beforeEach(() => { + inputHandler = new InputHandler(); + mockTdpClient = new TdpClient( + () => null, + selectDirectoryInBrowser + ) as jest.Mocked; + }); + + afterEach(() => { + inputHandler.dispose(); + }); + + describe('synchronizeModifierState', () => { + it('sends modifier sync when local and remote states differ', () => { + // Create event with Shift pressed but don't track it in remote state first + const event = new KeyboardEvent('keydown', { + code: 'KeyA', + shiftKey: true, + }); + + const params = { + e: event, + state: ButtonState.DOWN, + cli: mockTdpClient, + }; + + inputHandler.handleInputEvent(params); + + // Should send Shift DOWN to sync states since remote state defaults to UP + expect(mockTdpClient.sendKeyboardInput).toHaveBeenCalledWith( + 'ShiftLeft', + ButtonState.DOWN + ); + }); + + it('does not send sync when states are already synchronized', () => { + // First, set Shift as DOWN in remote state + const shiftDownEvent = new KeyboardEvent('keydown', { + code: 'ShiftLeft', + }); + inputHandler.handleInputEvent({ + e: shiftDownEvent, + state: ButtonState.DOWN, + cli: mockTdpClient, + }); + + mockTdpClient.sendKeyboardInput.mockClear(); + + // Now press a key with Shift + const event = new KeyboardEvent('keydown', { + code: 'KeyA', + shiftKey: true, + }); + + inputHandler.handleInputEvent({ + e: event, + state: ButtonState.DOWN, + cli: mockTdpClient, + }); + + // Should not send any Shift synchronization events + const shiftCalls = mockTdpClient.sendKeyboardInput.mock.calls.filter( + call => call[0].includes('Shift') + ); + expect(shiftCalls).toHaveLength(0); + }); + + it('synchronizes multiple modifier states correctly', () => { + // Set Alt as DOWN in remote state (to test it gets synced to UP) + const altDownEvent = new KeyboardEvent('keydown', { code: 'AltLeft' }); + inputHandler.handleInputEvent({ + e: altDownEvent, + state: ButtonState.DOWN, + cli: mockTdpClient, + }); + + // Press event with multiple modifiers active but not previously tracked + const event = new KeyboardEvent('keydown', { + code: 'KeyA', + shiftKey: true, + ctrlKey: true, + altKey: false, + metaKey: false, + }); + + inputHandler.handleInputEvent({ + e: event, + state: ButtonState.DOWN, + cli: mockTdpClient, + }); + + // Should sync Shift and Control to DOWN, and Alt to UP + expect(mockTdpClient.sendKeyboardInput).toHaveBeenCalledWith( + 'ShiftLeft', + ButtonState.DOWN + ); + expect(mockTdpClient.sendKeyboardInput).toHaveBeenCalledWith( + 'ControlLeft', + ButtonState.DOWN + ); + expect(mockTdpClient.sendKeyboardInput).toHaveBeenCalledWith( + 'AltLeft', + ButtonState.UP + ); + }); + + it('handles modifier key release correctly', () => { + // First press Shift + const shiftDownEvent = new KeyboardEvent('keydown', { + code: 'ShiftLeft', + }); + inputHandler.handleInputEvent({ + e: shiftDownEvent, + state: ButtonState.DOWN, + cli: mockTdpClient, + }); + + // Then release Shift + const shiftUpEvent = new KeyboardEvent('keyup', { code: 'ShiftLeft' }); + inputHandler.handleInputEvent({ + e: shiftUpEvent, + state: ButtonState.UP, + cli: mockTdpClient, + }); + + mockTdpClient.sendKeyboardInput.mockClear(); + + // Now press a key without Shift + const normalKeyEvent = new KeyboardEvent('keydown', { + code: 'KeyA', + shiftKey: false, + }); + + inputHandler.handleInputEvent({ + e: normalKeyEvent, + state: ButtonState.DOWN, + cli: mockTdpClient, + }); + + // Should not send additional Shift events since it's already UP + const shiftCalls = mockTdpClient.sendKeyboardInput.mock.calls.filter( + call => call[0].includes('Shift') + ); + expect(shiftCalls).toHaveLength(0); + }); + }); +}); diff --git a/web/packages/shared/components/DesktopSession/InputHandler.tsx b/web/packages/shared/components/DesktopSession/InputHandler.tsx index b14b037be1259..7f83cb5df7e70 100644 --- a/web/packages/shared/components/DesktopSession/InputHandler.tsx +++ b/web/packages/shared/components/DesktopSession/InputHandler.tsx @@ -37,6 +37,18 @@ export class InputHandler { private syncBeforeNextKey: boolean = true; private static isMac: boolean = getPlatform() === Platform.macOS; + /** + * Keep track of the remote state of modifier keys. + * This is used to mitigate stuck key issues when local + * and remote modifier states are out of sync. + */ + private remoteModifierState = new Map([ + ['Shift', ButtonState.UP], + ['Control', ButtonState.UP], + ['Alt', ButtonState.UP], + ['Meta', ButtonState.UP], + ]); + constructor() { // Bind finishHandlingInputEvent to this instance so it can be passed // as a callback to the Withholder. @@ -47,13 +59,14 @@ export class InputHandler { * Primary method for handling input events. */ public handleInputEvent(params: InputEventParams) { - const { e, cli } = params; + const { e, cli, state } = params; if (e instanceof KeyboardEvent) { // Only prevent default for KeyboardEvents. // If preventDefault is done on MouseEvents, // it breaks focus and keys won't be registered. e.preventDefault(); this.handleSyncBeforeNextKey(cli, e); + this.updateModifierState(e.code, state); } this.withholder.handleInputEvent(params, this.finishHandlingInputEvent); } @@ -100,6 +113,9 @@ export class InputHandler { private finishHandlingInputEvent(params: InputEventParams): void { const { cli, e, state } = params; + // Synchronize local and remote modifier state before sending the input event. + this.synchronizeModifierState(cli, e); + // If this is a mouse event no special handling is needed. if (e instanceof MouseEvent) { cli.sendMouseButton(e.button as MouseButton, state); @@ -118,6 +134,78 @@ export class InputHandler { } } + /** + * Updates the locally tracked remote modifier state. + * + * @param keyCode The key code of the key. + * @param state The state of the key. + */ + private updateModifierState(keyCode: string, state: ButtonState) { + switch (keyCode) { + case 'ShiftLeft': + case 'ShiftRight': + this.remoteModifierState.set('Shift', state); + break; + case 'ControlLeft': + case 'ControlRight': + this.remoteModifierState.set('Control', state); + break; + case 'AltLeft': + case 'AltRight': + this.remoteModifierState.set('Alt', state); + break; + case 'MetaLeft': + case 'MetaRight': + this.remoteModifierState.set('Meta', state); + break; + } + } + + /** + * Checks if the given key is a modifier key. + * + * @param key The key to check. + * @returns True if the key is a modifier key, false otherwise. + */ + private isModifierKey(key: string): boolean { + return ['Shift', 'Control', 'Alt', 'Meta'].some(modifier => + key.startsWith(modifier) + ); + } + + /** + * Synchronizes the local modifier state with the remote machine. + * This is called when key or mouse click/scroll events occur to ensure + * that the remote machine has the correct state of modifier keys. + * If not, it sends the current state. + * + * @param cli The TdpClient instance used to send the state. + * @param e The KeyboardEvent or MouseEvent that triggered the synchronization. + */ + public synchronizeModifierState( + cli: TdpClient, + e: KeyboardEvent | MouseEvent + ) { + // Don't process modifier keys themselves + if (e instanceof KeyboardEvent && this.isModifierKey(e.code)) { + return; + } + + this.remoteModifierState.forEach((state, modifier) => { + const localState = e.getModifierState(modifier) + ? ButtonState.DOWN + : ButtonState.UP; + + if (localState !== state) { + // If the local state is different from the remote state, send the updates. + cli.sendKeyboardInput(modifier + 'Left', localState); + cli.sendKeyboardInput(modifier + 'Right', localState); + // Update the remote state to match the local state. + this.remoteModifierState.set(modifier, localState); + } + }); + } + /** * Must be called when the element associated with the InputHandler loses focus. */