diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 3fd2c385..7ecc994c 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -308,6 +308,7 @@ Object.assign(CodemanApp.prototype, { const container = document.getElementById('terminalContainer'); this.terminal.open(container); this._installMobileTapMouseGuard(); + this._installShiftDragSelection(); this._installTouchSelectionFocusGuard(); // Let xterm's CompositionHelper own IME key events. In particular, a @@ -911,7 +912,25 @@ Object.assign(CodemanApp.prototype, { container.addEventListener('contextmenu', (ev) => { if (longPressTimer !== null || this._touchSelecting || this._touchSelectionActive) { ev.preventDefault(); + return; } + // Right-click COPIES the selection, the mintty/PuTTY convention, because + // the browser's own menu structurally cannot offer it here: xterm paints + // glyphs into a canvas, so a terminal selection is not a DOM selection + // and the native "Copy" item has nothing to act on (it is absent or + // inert). This is the second half of the habit users bring from a native + // terminal running a mouse-tracking TUI — Shift+drag to select (see + // _installShiftDragSelection), right-click to copy — and without it that + // gesture dead-ends after the selection is made. + // + // With NOTHING selected the native menu is left alone: it still carries + // the browser-level items (reload, inspect) and suppressing it there + // would take them away to offer nothing in return. + if (!this.terminal?.hasSelection?.()) return; + const selection = this.terminal.getSelection(); + if (!selection) return; + ev.preventDefault(); + void this.copyTerminalSelection(selection); }); container.addEventListener( @@ -4853,6 +4872,51 @@ Object.assign(CodemanApp.prototype, { this._sendSyntheticSgrTap(ev.clientX, ev.clientY); }, + /** + * Make Shift+drag START a selection instead of trying to extend one. + * + * In a native terminal running a mouse-tracking TUI (claude, codex), Shift is + * the "let me select text" modifier: it bypasses the app's mouse reporting so + * the emulator selects locally. Users bring that habit here, and here it did + * NOTHING — Shift+drag selected no text at all (measured). + * + * The reason is that the habit and xterm's Shift mean different things once + * the DECSETs are stripped. xterm reads Shift as "force selection" ONLY while + * the app actually has mouse tracking on; the server strips those DECSETs for + * claude/codex/gemini (isAltScreenStripMode), so xterm's mouseTrackingMode is + * permanently `none`, that branch is unreachable, and Shift instead falls into + * `_onIncrementalClick` — EXTEND an existing selection. Extending is a no-op + * when `selectionStart` is null, so the drag never anchors and no selection is + * ever built (this is why nothing gets cleared: there was nothing to clear). + * + * So plant the anchor xterm is missing. Runs in the CAPTURE phase on the + * `.xterm` root, an ancestor of the `.xterm-screen` element SelectionService + * binds to, so it lands before xterm's own mousedown; xterm's incremental + * handler then extends from our anchor and the drag behaves like a plain one. + * A Shift+drag with a selection ALREADY up is left alone — that is a genuine + * extend gesture and xterm already does it right. + */ + _installShiftDragSelection() { + const el = this.terminal?.element; + if (!el || el._codemanShiftDragInstalled) return; + el._codemanShiftDragInstalled = true; + el.addEventListener( + 'mousedown', + (ev) => { + if (!ev.isTrusted || ev.button !== 0 || !ev.shiftKey) return; + if (ev.altKey || ev.ctrlKey || ev.metaKey) return; + if (this.terminal?.hasSelection?.()) return; + const pos = this._clientPointToCell(ev.clientX, ev.clientY); + if (!pos) return; + // _clientPointToCell is 1-based and viewport-relative; select() takes a + // 0-based column and an ABSOLUTE buffer row. + const viewportY = this.terminal.buffer?.active?.viewportY ?? 0; + this.terminal.select(pos.col - 1, pos.row - 1 + viewportY, 0); + }, + true + ); + }, + _installMobileTapMouseGuard() { const el = this.terminal?.element; if (!el || el._codemanTapMouseGuardInstalled) return; diff --git a/test/terminal-shift-drag-selection.test.ts b/test/terminal-shift-drag-selection.test.ts new file mode 100644 index 00000000..ed196b19 --- /dev/null +++ b/test/terminal-shift-drag-selection.test.ts @@ -0,0 +1,168 @@ +/** + * Shift+drag must START a terminal selection, and right-click must copy it. + * + * Both halves come from one habit users bring from a native terminal running a + * mouse-tracking TUI (claude, codex): hold Shift to select locally, then + * right-click to copy. In Codeman both halves were broken, for two unrelated + * reasons, and the pair dead-ended the whole gesture. + */ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it, vi } from 'vitest'; + +function loadHarness() { + const CodemanApp = function CodemanApp(this: unknown) {}; + const windowRef: Record = {}; + const context = vm.createContext({ + window: windowRef, + document: { body: { classList: { contains: () => false } }, getElementById: () => null, activeElement: null }, + CodemanApp, + console: { warn: vi.fn(), log: vi.fn(), debug: vi.fn() }, + _crashDiag: { log: vi.fn() }, + performance: { now: () => 1000 }, + requestAnimationFrame: () => 1, + setTimeout: () => 1, + Blob: function Blob() {}, + URL: { createObjectURL: () => 'blob:x', revokeObjectURL: () => {} }, + Worker: function Worker(this: { postMessage: () => void }) { + this.postMessage = () => {}; + }, + MobileDetection: { isTouchDevice: () => false }, + KeyboardHandler: {}, + }); + vm.runInContext(readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8'), context); + vm.runInContext(readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8'), context); + return new (CodemanApp as unknown as new () => Record unknown>)(); +} + +/** A terminal whose element records the capture-phase mousedown listener we install. */ +function makeTerminal(opts: { hasSelection: boolean; viewportY: number }) { + const selectCalls: Array<[number, number, number]> = []; + let listener: ((ev: Record) => void) | null = null; + const terminal = { + element: { + addEventListener: (type: string, fn: (ev: Record) => void, capture: boolean) => { + // Capture on the `.xterm` root is what puts us ahead of xterm's own + // mousedown on the descendant `.xterm-screen`; a bubble-phase listener + // would run after SelectionService had already decided. + expect(type).toBe('mousedown'); + expect(capture).toBe(true); + listener = fn; + }, + querySelector: () => ({ getBoundingClientRect: () => ({ left: 0, top: 0 }) }), + }, + cols: 80, + rows: 24, + buffer: { active: { viewportY: opts.viewportY } }, + hasSelection: () => opts.hasSelection, + select: (c: number, r: number, l: number) => selectCalls.push([c, r, l]), + _core: { _renderService: { dimensions: { css: { cell: { width: 10, height: 20 } } } } }, + }; + return { terminal, selectCalls, fire: (ev: Record) => listener?.(ev) }; +} + +const mousedown = (over: Record = {}) => ({ + isTrusted: true, + button: 0, + shiftKey: true, + altKey: false, + ctrlKey: false, + metaKey: false, + clientX: 35, + clientY: 50, + ...over, +}); + +describe('Shift+drag starts a selection', () => { + it('plants the anchor xterm never sets, in 0-based ABSOLUTE buffer coordinates', () => { + // xterm reads Shift as "force selection" only while the app has mouse + // tracking on, and the server strips those DECSETs for claude/codex/gemini, + // so Shift instead reaches `_onIncrementalClick` — extend — which is a no-op + // with no selectionStart. The drag then anchors nothing and selects NOTHING. + const app = loadHarness(); + const { terminal, selectCalls, fire } = makeTerminal({ hasSelection: false, viewportY: 120 }); + app.terminal = terminal as never; + app._installShiftDragSelection(); + fire(mousedown()); + // clientX 35 / 10px cells -> viewport col 4 (1-based) -> column 3 (0-based). + // clientY 50 / 20px cells -> viewport row 3 (1-based) -> row 2, + viewportY. + expect(selectCalls).toEqual([[3, 2 + 120, 0]]); + }); + + it('uses length 0 so a Shift+CLICK that never drags selects nothing', () => { + const app = loadHarness(); + const { terminal, selectCalls, fire } = makeTerminal({ hasSelection: false, viewportY: 0 }); + app.terminal = terminal as never; + app._installShiftDragSelection(); + fire(mousedown()); + expect(selectCalls[0][2]).toBe(0); + }); + + it('leaves a real EXTEND gesture to xterm when a selection is already up', () => { + const app = loadHarness(); + const { terminal, selectCalls, fire } = makeTerminal({ hasSelection: true, viewportY: 0 }); + app.terminal = terminal as never; + app._installShiftDragSelection(); + fire(mousedown()); + expect(selectCalls).toEqual([]); + }); + + it('ignores everything that is not a trusted plain-Shift left press', () => { + const app = loadHarness(); + const { terminal, selectCalls, fire } = makeTerminal({ hasSelection: false, viewportY: 0 }); + app.terminal = terminal as never; + app._installShiftDragSelection(); + for (const ev of [ + mousedown({ shiftKey: false }), + mousedown({ button: 2 }), + mousedown({ isTrusted: false }), + mousedown({ ctrlKey: true }), + mousedown({ altKey: true }), + mousedown({ metaKey: true }), + ]) { + fire(ev); + } + expect(selectCalls).toEqual([]); + }); + + it('installs exactly once per terminal element', () => { + const app = loadHarness(); + const { terminal } = makeTerminal({ hasSelection: false, viewportY: 0 }); + let installs = 0; + terminal.element.addEventListener = () => { + installs += 1; + }; + app.terminal = terminal as never; + app._installShiftDragSelection(); + app._installShiftDragSelection(); + expect(installs).toBe(1); + }); +}); + +describe('right-click copies the terminal selection', () => { + // The handler is created inline inside initTerminal (it closes over the + // long-press timer it shares with the touch path), so this pins the ordering + // in the shipped source: gesture suppression first, then the no-selection + // early return, and only then the copy. + const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8'); + const handler = source.slice( + source.indexOf("container.addEventListener('contextmenu'"), + source.indexOf("container.addEventListener('touchcancel'") + ); + + it('suppresses the menu during a touch selection gesture and stops there', () => { + expect(handler).toMatch(/longPressTimer !== null[\s\S]*?ev\.preventDefault\(\);\s*return;/); + }); + + it('returns BEFORE preventDefault when nothing is selected, keeping the native menu', () => { + const noSelection = handler.indexOf('hasSelection'); + const prevent = handler.lastIndexOf('ev.preventDefault()'); + expect(noSelection).toBeGreaterThan(-1); + expect(noSelection).toBeLessThan(prevent); + }); + + it('copies through copyTerminalSelection rather than a second clipboard path', () => { + expect(handler).toContain('this.copyTerminalSelection(selection)'); + }); +});