From fc10bc4cc02a3c5c70767547be7205accbd701e4 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Fri, 29 May 2026 22:44:57 -0400 Subject: [PATCH] Fix mobile terminal tap mouse reports Co-authored-by: Orca --- .../app/h/[hostId]/session/[worktreeId].tsx | 10 ++-- mobile/src/terminal/TerminalWebView.tsx | 54 ++++++++++++++++++- .../terminal/terminal-gesture-input.test.ts | 24 +++++++++ mobile/src/terminal/terminal-gesture-input.ts | 36 ++++++++++--- .../terminal-webview-scroll-routing.test.ts | 53 ++++++++++++++++++ 5 files changed, 164 insertions(+), 13 deletions(-) diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 17f163529..26d42bab2 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -352,6 +352,10 @@ function isWheelMouseTrackingMode(mode: TerminalModes['mouseTrackingMode'] | und return mode === 'vt200' || mode === 'drag' || mode === 'any' } +function isGestureMouseTrackingMode(mode: TerminalModes['mouseTrackingMode'] | undefined): boolean { + return mode === 'x10' || isWheelMouseTrackingMode(mode) +} + function TerminalPaneView({ handle, active, @@ -2469,9 +2473,9 @@ export default function SessionScreen() { if (handle !== activeHandleRef.current || activeSessionTabTypeRef.current !== 'terminal') return const modes = ptyModesRef.current.get(handle) - // Why: WebView messages can become PTY input here. Only TUI scroll paths - // generate gesture input, and the bridge is rate-limited for SSH safety. - if (!modes?.altScreen && !isWheelMouseTrackingMode(modes?.mouseTrackingMode)) return + // Why: WebView gesture bytes can become PTY input here, so mouse-aware + // reports stay behind validation and SSH-safe rate limiting. + if (!modes?.altScreen && !isGestureMouseTrackingMode(modes?.mouseTrackingMode)) return const sequenceCount = countTerminalGestureInputSequences(bytes) if (sequenceCount == null) return if (!allowTerminalGestureInput(handle, sequenceCount)) return diff --git a/mobile/src/terminal/TerminalWebView.tsx b/mobile/src/terminal/TerminalWebView.tsx index 4d0b12eb6..99771f3e4 100644 --- a/mobile/src/terminal/TerminalWebView.tsx +++ b/mobile/src/terminal/TerminalWebView.tsx @@ -1076,11 +1076,15 @@ const XTERM_HTML = ` if (!cell) return ''; var eventCode = lines < 0 ? 64 : 65; if (sgrMousePixelsMode) { + if (!isSafeSgrMouseCoordinate(cell.x) || !isSafeSgrMouseCoordinate(cell.y)) return ''; return ESC + '[<' + eventCode + ';' + cell.x + ';' + cell.y + 'M'; } if (sgrMouseMode) { // Why: xterm increments zero-based mouse cells before encoding reports. - return ESC + '[<' + eventCode + ';' + (cell.col + 1) + ';' + (cell.row + 1) + 'M'; + var sgrCol = cell.col + 1; + var sgrRow = cell.row + 1; + if (!isSafeSgrMouseCoordinate(sgrCol) || !isSafeSgrMouseCoordinate(sgrRow)) return ''; + return ESC + '[<' + eventCode + ';' + sgrCol + ';' + sgrRow + 'M'; } // Why: xterm increments zero-based mouse cells before encoding reports. var button = eventCode + 32; @@ -1092,6 +1096,47 @@ const XTERM_HTML = ` return ESC + '[M' + String.fromCharCode(button) + String.fromCharCode(col) + String.fromCharCode(row); } + function isSafeSgrMouseCoordinate(value) { + return Number.isInteger(value) && value >= 0 && value <= 9999; + } + + function buildMouseClickInput(clientX, clientY) { + var mouseTrackingMode = getMouseTrackingMode(); + if (!isClickMouseTrackingMode(mouseTrackingMode)) return ''; + var cell = viewportToMouseReportCell(clientX, clientY); + if (!cell) return ''; + if (sgrMousePixelsMode) { + // Why: xterm 1016 keeps SGR syntax but reports raw zero-based pixel positions. + var pixelX = cell.x; + var pixelY = cell.y; + if (!isSafeSgrMouseCoordinate(pixelX) || !isSafeSgrMouseCoordinate(pixelY)) return ''; + var pixelPress = ESC + '[<0;' + pixelX + ';' + pixelY + 'M'; + if (mouseTrackingMode === 'x10') return pixelPress; + return pixelPress + ESC + '[<0;' + pixelX + ';' + pixelY + 'm'; + } + if (sgrMouseMode) { + // Why: xterm increments zero-based mouse cells before encoding reports. + var sgrCol = cell.col + 1; + var sgrRow = cell.row + 1; + if (!isSafeSgrMouseCoordinate(sgrCol) || !isSafeSgrMouseCoordinate(sgrRow)) return ''; + var sgrPress = ESC + '[<0;' + sgrCol + ';' + sgrRow + 'M'; + if (mouseTrackingMode === 'x10') return sgrPress; + return sgrPress + ESC + '[<0;' + sgrCol + ';' + sgrRow + 'm'; + } + // Why: non-SGR click coordinates use printable ASCII bytes on the mobile + // bridge; unsafe wide-terminal cells must not turn into corrupted input. + var col = cell.col + 1 + 32; + var row = cell.row + 1 + 32; + if (col > 126 || row > 126) return ''; + var press = ESC + '[M' + String.fromCharCode(32) + String.fromCharCode(col) + String.fromCharCode(row); + if (mouseTrackingMode === 'x10') return press; + return press + ESC + '[M' + String.fromCharCode(35) + String.fromCharCode(col) + String.fromCharCode(row); + } + + function isClickMouseTrackingMode(mode) { + return mode !== 'none'; + } + function isWheelMouseTrackingMode(mode) { return mode !== 'none' && mode !== 'x10'; } @@ -1561,7 +1606,12 @@ const XTERM_HTML = ` } if (dispatch.mode === 'surface') { if (e.touches.length === 0 && longPressOrigin && selMode !== 'select') { - notify({ type: 'terminal-tap' }); + var clickInput = buildMouseClickInput(longPressOrigin.x, longPressOrigin.y); + if (clickInput) { + notify({ type: 'terminal-input', bytes: clickInput }); + } else if (!isClickMouseTrackingMode(getMouseTrackingMode())) { + notify({ type: 'terminal-tap' }); + } } clearLongPress(); if (e.touches.length === 0) { diff --git a/mobile/src/terminal/terminal-gesture-input.test.ts b/mobile/src/terminal/terminal-gesture-input.test.ts index 48ad19e10..a120d569d 100644 --- a/mobile/src/terminal/terminal-gesture-input.test.ts +++ b/mobile/src/terminal/terminal-gesture-input.test.ts @@ -19,6 +19,14 @@ describe('isTerminalGestureInput', () => { expect(isTerminalGestureInput(`${ESC}[<64;0;0M`)).toBe(true) }) + it('accepts bounded SGR left-click press and release sequences', () => { + expect(isTerminalGestureInput(`${ESC}[<0;38;20M${ESC}[<0;38;20m`)).toBe(true) + expect(countTerminalGestureInputSequences(`${ESC}[<0;38;20M${ESC}[<0;38;20m`)).toBe(2) + expect(countTerminalGestureInputSequences(`${ESC}[<0;1;1M${ESC}[<0;1;1m`)).toBe(2) + expect(countTerminalGestureInputSequences(`${ESC}[<0;0;0M${ESC}[<0;0;0m`)).toBe(2) + expect(isTerminalGestureInput(`${ESC}[<0;9999;9999M${ESC}[<0;9999;9999m`)).toBe(true) + }) + it('accepts repeated default mouse wheel sequences', () => { expect( isTerminalGestureInput( @@ -27,6 +35,14 @@ describe('isTerminalGestureInput', () => { ).toBe(true) }) + it('accepts bounded default left-click press and release sequences', () => { + expect( + isTerminalGestureInput( + `${ESC}[M${String.fromCharCode(32, 33, 33)}${ESC}[M${String.fromCharCode(35, 33, 33)}` + ) + ).toBe(true) + }) + it('rejects shell text and other terminal input', () => { expect(isTerminalGestureInput('rm -rf .\r')).toBe(false) expect(isTerminalGestureInput(`${ESC}[200~paste${ESC}[201~`)).toBe(false) @@ -39,4 +55,12 @@ describe('isTerminalGestureInput', () => { expect(countTerminalGestureInputSequences(`${ESC}[A`.repeat(33))).toBeNull() expect(isTerminalGestureInput(`${ESC}[A`.repeat(700))).toBe(false) }) + + it('rejects malformed SGR click reports and wheel releases', () => { + expect(isTerminalGestureInput(`${ESC}[<0;;1M`)).toBe(false) + expect(isTerminalGestureInput(`${ESC}[<0;-1;1M`)).toBe(false) + expect(isTerminalGestureInput(`${ESC}[<0;10000;1M`)).toBe(false) + expect(isTerminalGestureInput(`${ESC}[<64;1;1m`)).toBe(false) + expect(isTerminalGestureInput(`${ESC}[<65;1;1m`)).toBe(false) + }) }) diff --git a/mobile/src/terminal/terminal-gesture-input.ts b/mobile/src/terminal/terminal-gesture-input.ts index 15b3a9d11..83450a173 100644 --- a/mobile/src/terminal/terminal-gesture-input.ts +++ b/mobile/src/terminal/terminal-gesture-input.ts @@ -1,31 +1,51 @@ const ESC = '\x1b' const MAX_TERMINAL_GESTURE_INPUT_LENGTH = 2048 const MAX_TERMINAL_GESTURE_INPUT_SEQUENCES = 32 -const SGR_MOUSE_WHEEL_SEQUENCE_RE = new RegExp(`^${ESC}\\[<(64|65);[0-9]{1,4};[0-9]{1,4}M$`) +const SGR_MOUSE_GESTURE_SEQUENCE_RE = new RegExp( + `^${ESC}\\[<(0|64|65);([0-9]{1,4});([0-9]{1,4})([Mm])$` +) -function isDefaultMouseWheelSequence(bytes: string, offset: number): number | null { +function isDefaultMouseGestureSequence(bytes: string, offset: number): number | null { if (!bytes.startsWith(`${ESC}[M`, offset) || offset + 6 > bytes.length) { return null } const button = bytes.charCodeAt(offset + 3) const col = bytes.charCodeAt(offset + 4) const row = bytes.charCodeAt(offset + 5) - if ((button === 96 || button === 97) && col >= 33 && col <= 126 && row >= 33 && row <= 126) { + if ( + (button === 32 || button === 35 || button === 96 || button === 97) && + col >= 33 && + col <= 126 && + row >= 33 && + row <= 126 + ) { return offset + 6 } return null } -function isSgrMouseWheelSequence(bytes: string, offset: number): number | null { +function isSgrMouseGestureSequence(bytes: string, offset: number): number | null { if (!bytes.startsWith(`${ESC}[<`, offset)) { return null } - const end = bytes.indexOf('M', offset) + const pressEnd = bytes.indexOf('M', offset) + const releaseEnd = bytes.indexOf('m', offset) + const end = + pressEnd === -1 ? releaseEnd : releaseEnd === -1 ? pressEnd : Math.min(pressEnd, releaseEnd) if (end === -1) { return null } const sequence = bytes.slice(offset, end + 1) - return SGR_MOUSE_WHEEL_SEQUENCE_RE.test(sequence) ? end + 1 : null + const match = SGR_MOUSE_GESTURE_SEQUENCE_RE.exec(sequence) + if (!match) return null + const button = match[1] + const col = Number(match[2]) + const row = Number(match[3]) + const final = match[4] + if (button === '0') { + return col >= 0 && row >= 0 ? end + 1 : null + } + return final === 'M' ? end + 1 : null } function isArrowScrollSequence(bytes: string, offset: number): number | null { @@ -51,8 +71,8 @@ export function countTerminalGestureInputSequences(bytes: string): number | null while (offset < bytes.length) { const next = isArrowScrollSequence(bytes, offset) ?? - isSgrMouseWheelSequence(bytes, offset) ?? - isDefaultMouseWheelSequence(bytes, offset) + isSgrMouseGestureSequence(bytes, offset) ?? + isDefaultMouseGestureSequence(bytes, offset) if (next == null) { return null diff --git a/mobile/src/terminal/terminal-webview-scroll-routing.test.ts b/mobile/src/terminal/terminal-webview-scroll-routing.test.ts index 3d57a01fe..9aabc65a6 100644 --- a/mobile/src/terminal/terminal-webview-scroll-routing.test.ts +++ b/mobile/src/terminal/terminal-webview-scroll-routing.test.ts @@ -2,6 +2,10 @@ import { readFileSync } from 'node:fs' import { describe, expect, it } from 'vitest' const source = readFileSync(new URL('./TerminalWebView.tsx', import.meta.url), 'utf8') +const sessionSource = readFileSync( + new URL('../../app/h/[hostId]/session/[worktreeId].tsx', import.meta.url), + 'utf8' +) function sliceBetween(startPattern: string, endPattern: string): string { const start = source.indexOf(startPattern) @@ -125,4 +129,53 @@ describe('TerminalWebView scroll routing', () => { expect(dragMoveBlock).toContain('edgeScrollClientY = clientY;') expect(dragMoveBlock).toContain('syncSelectionHandleToViewportPoint(handle, clientX, clientY)') }) + + it('synthesizes bounded mouse clicks from surface taps before focus fallback', () => { + expect(source).toContain('function buildMouseClickInput(clientX, clientY)') + expect(source).toContain('function isClickMouseTrackingMode(mode)') + expect(source).toContain("return mode !== 'none';") + expect(source).toContain('var pixelX = cell.x;') + expect(source).toContain('var pixelY = cell.y;') + expect(source).toContain( + 'if (!isSafeSgrMouseCoordinate(cell.x) || !isSafeSgrMouseCoordinate(cell.y)) return' + ) + expect(source).toContain( + 'if (!isSafeSgrMouseCoordinate(sgrCol) || !isSafeSgrMouseCoordinate(sgrRow)) return' + ) + expect(source).toContain("if (mouseTrackingMode === 'x10') return pixelPress;") + expect(source).toContain("if (mouseTrackingMode === 'x10') return sgrPress;") + expect(source).toContain("if (mouseTrackingMode === 'x10') return press;") + expect(source).toContain("if (col > 126 || row > 126) return '';") + + const touchEndBlock = sliceBetween( + "document.addEventListener('touchend'", + '}, { capture: true, passive: true });' + ) + expect(touchEndBlock.indexOf('var clickInput = buildMouseClickInput')).toBeLessThan( + touchEndBlock.indexOf("notify({ type: 'terminal-tap' });") + ) + expect(touchEndBlock).toContain("notify({ type: 'terminal-input', bytes: clickInput });") + expect(touchEndBlock).toContain( + '} else if (!isClickMouseTrackingMode(getMouseTrackingMode())) {' + ) + }) + + it('allows x10 mouse gesture reports through the mobile session gate', () => { + expect(sessionSource).toContain('function isGestureMouseTrackingMode') + expect(sessionSource).toContain("return mode === 'x10' || isWheelMouseTrackingMode(mode)") + + const inputBlockStart = sessionSource.indexOf('const handleTerminalInput = useCallback') + expect(inputBlockStart).toBeGreaterThanOrEqual(0) + const inputBlockEnd = sessionSource.indexOf( + 'async function handleClearTerminal', + inputBlockStart + ) + expect(inputBlockEnd).toBeGreaterThan(inputBlockStart) + const inputBlock = sessionSource.slice(inputBlockStart, inputBlockEnd) + expect(inputBlock).toContain('!isGestureMouseTrackingMode(modes?.mouseTrackingMode)') + expect(inputBlock).toContain('const sequenceCount = countTerminalGestureInputSequences(bytes)') + expect(inputBlock.indexOf('countTerminalGestureInputSequences')).toBeLessThan( + inputBlock.indexOf('enqueueTerminalGestureInput') + ) + }) })