From bb2fba4fb3c341592c54e1f3897ac6cd946fb5be Mon Sep 17 00:00:00 2001 From: Elad Avraham Date: Mon, 4 May 2026 18:49:58 +0300 Subject: [PATCH] fix: suppress right-click paste after failed drag-select with mouse reporting When a TUI app enables mouse reporting, xterm forwards drag events to the pty as SGR mouse sequences instead of creating a DOM selection. The user thinks they selected text, but hasSelection() returns false causing the right-click handler to paste clipboard content into the terminal. Fix: track left-button drag attempts. When a drag occurred with mouse tracking active and no xterm selection resulted, set a short-lived flag (recentDragWithoutSelection) that suppresses the paste on the subsequent right-click. This is narrower than blocking all paste when mouse reporting is on (which breaks PSReadLine's click-to-position since it enables ?1003h). The flag auto-expires after 3s so intentional TUI mouse interactions don't permanently disable paste. Users can always Ctrl+V to paste explicitly. Applied to both TerminalPanel and DetachedApp. Added e2e tests verifying both the fix and no-regression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/renderer/DetachedApp.tsx | 52 +++++- src/renderer/components/TerminalPanel.tsx | 42 +++++ ...htclick-paste-mouse-reporting-text.spec.ts | 150 ++++++++++++++++++ 3 files changed, 243 insertions(+), 1 deletion(-) create mode 100644 tests/e2e/rightclick-paste-mouse-reporting-text.spec.ts diff --git a/src/renderer/DetachedApp.tsx b/src/renderer/DetachedApp.tsx index 7233cb7d..6012fb46 100644 --- a/src/renderer/DetachedApp.tsx +++ b/src/renderer/DetachedApp.tsx @@ -114,8 +114,21 @@ const DetachedApp: React.FC = ({ terminalId }) => { window.terminalAPI.writePty(terminalId, data); }); + let mouseTrackingOn = false; const unsubscribePtyData = window.terminalAPI.onPtyData((id, data) => { - if (id === terminalId) term.write(data); + if (id !== terminalId) return; + // Track mouse reporting modes so handleContextMenu can suppress paste + for (let i = 0; i < data.length; i++) { + if (data.charCodeAt(i) !== 0x1b) continue; + if (data.startsWith('\x1b[?1000h', i) || data.startsWith('\x1b[?1002h', i) || + data.startsWith('\x1b[?1003h', i)) { + mouseTrackingOn = true; + } else if (data.startsWith('\x1b[?1000l', i) || data.startsWith('\x1b[?1002l', i) || + data.startsWith('\x1b[?1003l', i)) { + mouseTrackingOn = false; + } + } + term.write(data); }); const unsubscribePtyExit = window.terminalAPI.onPtyExit((id) => { @@ -140,6 +153,33 @@ const DetachedApp: React.FC = ({ terminalId }) => { // TerminalPanel so detached windows match main window behaviour. // Skip the implicit paste when clipboard is image-only (issue #84) - // see TerminalPanel.handleContextMenu for the rationale. + + // Track recent left-button drag attempts for failed-selection detection. + let recentDragWithoutSelection = false; + let dragStartPos: { x: number; y: number } | null = null; + const DRAG_THRESHOLD = 5; + + const handleLeftMouseDown = (e: MouseEvent) => { + if (e.button === 0) { + dragStartPos = { x: e.clientX, y: e.clientY }; + recentDragWithoutSelection = false; + } + }; + const handleLeftMouseUp = (e: MouseEvent) => { + if (e.button === 0 && dragStartPos) { + const dx = e.clientX - dragStartPos.x; + const dy = e.clientY - dragStartPos.y; + const wasDrag = Math.sqrt(dx * dx + dy * dy) > DRAG_THRESHOLD; + if (wasDrag && mouseTrackingOn && !term.hasSelection()) { + recentDragWithoutSelection = true; + setTimeout(() => { recentDragWithoutSelection = false; }, 3000); + } else { + recentDragWithoutSelection = false; + } + dragStartPos = null; + } + }; + const handleContextMenu = (e: MouseEvent) => { e.preventDefault(); e.stopPropagation(); @@ -149,6 +189,12 @@ const DetachedApp: React.FC = ({ terminalId }) => { term.clearSelection(); return; } + // When mouse tracking consumed a recent drag (user tried to select but + // the TUI swallowed it), suppress paste. + if (recentDragWithoutSelection) { + recentDragWithoutSelection = false; + return; + } const hasImage = window.terminalAPI.clipboardHasImage(); const html = window.terminalAPI.clipboardReadHTML(); const plainText = window.terminalAPI.clipboardRead(); @@ -176,6 +222,8 @@ const DetachedApp: React.FC = ({ terminalId }) => { containerEl.addEventListener('contextmenu', handleContextMenu, true); containerEl.addEventListener('mousedown', handleRightMouseButton, true); containerEl.addEventListener('mouseup', handleRightMouseButton, true); + containerEl.addEventListener('mousedown', handleLeftMouseDown, true); + containerEl.addEventListener('mouseup', handleLeftMouseUp, true); term.focus(); @@ -188,6 +236,8 @@ const DetachedApp: React.FC = ({ terminalId }) => { containerEl.removeEventListener('contextmenu', handleContextMenu, true); containerEl.removeEventListener('mousedown', handleRightMouseButton, true); containerEl.removeEventListener('mouseup', handleRightMouseButton, true); + containerEl.removeEventListener('mousedown', handleLeftMouseDown, true); + containerEl.removeEventListener('mouseup', handleLeftMouseUp, true); term.dispose(); }; })(); diff --git a/src/renderer/components/TerminalPanel.tsx b/src/renderer/components/TerminalPanel.tsx index 41541c09..f4a6eec9 100644 --- a/src/renderer/components/TerminalPanel.tsx +++ b/src/renderer/components/TerminalPanel.tsx @@ -1565,6 +1565,39 @@ const TerminalPanel: React.FC = ({ terminalId, floatTitleBar containerRef.current.addEventListener('mousedown', handleRightMouseButton, true); containerRef.current.addEventListener('mouseup', handleRightMouseButton, true); + // Track recent left-button drag attempts. When mouse reporting is on, + // xterm forwards the drag to the pty instead of creating a selection. + // We detect this "failed selection" so the right-click handler can avoid + // auto-pasting when the user clearly intended to copy, not paste. + let recentDragWithoutSelection = false; + let dragStartPos: { x: number; y: number } | null = null; + const DRAG_THRESHOLD = 5; // pixels to count as a drag vs. a click + + const handleLeftMouseDown = (e: MouseEvent) => { + if (e.button === 0) { + dragStartPos = { x: e.clientX, y: e.clientY }; + recentDragWithoutSelection = false; + } + }; + const handleLeftMouseUp = (e: MouseEvent) => { + if (e.button === 0 && dragStartPos) { + const dx = e.clientX - dragStartPos.x; + const dy = e.clientY - dragStartPos.y; + const wasDrag = Math.sqrt(dx * dx + dy * dy) > DRAG_THRESHOLD; + if (wasDrag && mouseTrackingOn && !term.hasSelection()) { + recentDragWithoutSelection = true; + // Auto-clear after a timeout — if the user doesn't right-click + // within 3s, the drag was probably intentional TUI interaction. + setTimeout(() => { recentDragWithoutSelection = false; }, 3000); + } else { + recentDragWithoutSelection = false; + } + dragStartPos = null; + } + }; + containerRef.current.addEventListener('mousedown', handleLeftMouseDown, true); + containerRef.current.addEventListener('mouseup', handleLeftMouseUp, true); + // Right-click: copy if selection, paste if no selection. // Skip the implicit paste when the clipboard is image-only (issue #84): // a TUI with mouse reporting on (e.g. Claude Code) consumes drag events @@ -1581,6 +1614,13 @@ const TerminalPanel: React.FC = ({ terminalId, floatTitleBar term.clearSelection(); return; } + // When mouse tracking consumed a recent drag (user tried to select but + // the TUI swallowed it), suppress paste. The user clearly intended to + // copy, not paste. Ctrl+V still pastes explicitly. + if (recentDragWithoutSelection) { + recentDragWithoutSelection = false; + return; + } const hasImage = window.terminalAPI.clipboardHasImage(); const html = window.terminalAPI.clipboardReadHTML(); const plainText = window.terminalAPI.clipboardRead(); @@ -1637,6 +1677,8 @@ const TerminalPanel: React.FC = ({ terminalId, floatTitleBar containerEl.removeEventListener('copy', handleCopyEvent, true); containerEl.removeEventListener('mousedown', handleRightMouseButton, true); containerEl.removeEventListener('mouseup', handleRightMouseButton, true); + containerEl.removeEventListener('mousedown', handleLeftMouseDown, true); + containerEl.removeEventListener('mouseup', handleLeftMouseUp, true); wheelRecoveryEl?.removeEventListener('wheel', wheelPreSyncHandler, true); wheelRecoveryEl?.removeEventListener('wheel', wheelRecoveryHandler); wheelRecoveryEl?.removeEventListener('dblclick', manualSyncHandler); diff --git a/tests/e2e/rightclick-paste-mouse-reporting-text.spec.ts b/tests/e2e/rightclick-paste-mouse-reporting-text.spec.ts new file mode 100644 index 00000000..cb2c32f5 --- /dev/null +++ b/tests/e2e/rightclick-paste-mouse-reporting-text.spec.ts @@ -0,0 +1,150 @@ +// Regression test: right-click in a terminal with mouse reporting active +// and text on the clipboard must NOT auto-paste. When mouse reporting is on, +// drag-select is consumed by the pty (forwarded as SGR events), so xterm +// never creates a selection. The user thinks they selected text, but +// hasSelection() is false — causing the paste branch to fire erroneously. +// +// This test verifies the fix: when mouseTrackingOn is true and there is no +// xterm selection, right-click is a no-op (user can still Ctrl+V explicitly). +import { test, expect, Page } from '@playwright/test'; +import { launchTmax } from './fixtures/launch'; + +async function installPtyWriteSpy(window: Page): Promise { + await window.evaluate(() => { + (window as any).__ptyWrites = []; + const orig = (window as any).terminalAPI.writePty.bind((window as any).terminalAPI); + (window as any).terminalAPI.writePty = (id: string, data: string) => { + (window as any).__ptyWrites.push({ id, data }); + return orig(id, data); + }; + }); +} + +async function getPastedText(window: Page): Promise { + const writes = await window.evaluate(() => (window as any).__ptyWrites.slice() as Array<{ id: string; data: string }>); + return writes.map(w => w.data).join(''); +} + +async function writeToTerminal(window: Page, text: string): Promise { + await window.evaluate((t: string) => { + const id = (window as any).__terminalStore.getState().focusedTerminalId; + const entry = (window as any).__getTerminalEntry(id); + entry?.terminal.write(t); + }, text); +} + +async function setClipboard(window: Page, text: string): Promise { + await window.evaluate((t: string) => (window as any).terminalAPI.clipboardWrite(t), text); +} + +test('right-click with mouse reporting on and text clipboard does NOT paste', async () => { + const { window, close } = await launchTmax(); + try { + await window.waitForSelector('.terminal-panel', { timeout: 15_000 }); + await window.waitForTimeout(800); + await installPtyWriteSpy(window); + + // Enable SGR mouse reporting (?1000h ?1006h) as a TUI app would + await writeToTerminal(window, '\x1b[?1000h\x1b[?1006h\r\nSELECTABLE_TEXT\r\n'); + await window.waitForTimeout(300); + + // Focus the terminal + await window.click('.terminal-panel .xterm-screen'); + await window.waitForTimeout(200); + + // Put text on the clipboard (simulates a previous copy from elsewhere) + const CLIPBOARD_PAYLOAD = 'SHOULD_NOT_BE_PASTED'; + await setClipboard(window, CLIPBOARD_PAYLOAD); + await window.waitForTimeout(100); + + // Drag across the terminal text — with mouse reporting on, xterm forwards + // the drag to the pty as SGR events and does NOT create a selection. + const coords = await window.evaluate(() => { + const id = (window as any).__terminalStore.getState().focusedTerminalId; + const entry = (window as any).__getTerminalEntry(id); + const term = entry?.terminal; + const dim = (term as any)._core?._renderService?.dimensions; + const cw = dim?.css?.cell?.width ?? dim?.actualCellWidth ?? 9; + const ch = dim?.css?.cell?.height ?? dim?.actualCellHeight ?? 18; + const screen = document.querySelector('.terminal-panel .xterm-screen') as HTMLElement; + const rect = screen.getBoundingClientRect(); + const startX = rect.left + 1; + const endX = rect.left + cw * 14 + cw * 0.6; + const y = rect.top + ch * 1.5; + return { startX, endX, y }; + }); + + await window.mouse.move(coords.startX, coords.y); + await window.mouse.down(); + await window.mouse.move(coords.endX, coords.y, { steps: 10 }); + await window.mouse.up(); + await window.waitForTimeout(250); + + // Confirm xterm has no selection (mouse reporting swallowed the drag) + const sel = await window.evaluate(() => { + const id = (window as any).__terminalStore.getState().focusedTerminalId; + const entry = (window as any).__getTerminalEntry(id); + return entry?.terminal.hasSelection() ?? false; + }); + expect(sel, 'mouse reporting should consume drag — no xterm selection').toBe(false); + + // Reset spy to only track what right-click does + await window.evaluate(() => { (window as any).__ptyWrites = []; }); + + // Right-click — should NOT paste the clipboard text + await window.click('.terminal-panel .xterm-screen', { button: 'right' }); + await window.waitForTimeout(500); + + const pasted = await getPastedText(window); + expect( + pasted, + `pty must not receive clipboard text on right-click when mouse reporting is on; got: ${JSON.stringify(pasted)}` + ).toBe(''); + } finally { + await close(); + } +}); + +test('right-click with mouse reporting OFF still pastes normally', async () => { + const { window, close } = await launchTmax(); + try { + await window.waitForSelector('.terminal-panel', { timeout: 15_000 }); + await window.waitForTimeout(1000); + + // No mouse reporting — normal terminal (PSReadLine may enable ?1003h, + // but with no preceding drag, paste should still fire) + + // Focus terminal + await window.click('.terminal-panel .xterm-screen'); + await window.waitForTimeout(200); + + const CLIPBOARD_PAYLOAD = 'PASTE_ME_E2E'; + await setClipboard(window, CLIPBOARD_PAYLOAD); + await window.waitForTimeout(100); + + // Right-click with no selection should paste + await window.click('.terminal-panel .xterm-screen', { button: 'right' }); + await window.waitForTimeout(1000); + + // Verify by reading the terminal buffer — the payload should be visible + const bufferText = await window.evaluate(() => { + const id = (window as any).__terminalStore.getState().focusedTerminalId; + const entry = (window as any).__getTerminalEntry(id); + const term = entry?.terminal; + if (!term) return ''; + const lines: string[] = []; + for (let i = 0; i < term.buffer.active.length; i++) { + const line = term.buffer.active.getLine(i)?.translateToString(true) ?? ''; + if (line.trim()) lines.push(line); + } + return lines.join('\n'); + }); + + expect( + bufferText.includes(CLIPBOARD_PAYLOAD), + `terminal buffer should contain pasted text; got: ${JSON.stringify(bufferText.slice(-200))}` + ).toBe(true); + } finally { + await close(); + } +});