diff --git a/docs/specs/layout.md b/docs/specs/layout.md index 513f9818a..0aef8cd30 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -208,11 +208,11 @@ Enter passthrough mode: clicking any pane body or header; `Enter` or `z` on a se Enter command mode: Left Cmd, then Right Cmd in quick succession — or the same left-then-right gesture with Shift — detected even while xterm holds DOM focus. -- The Meta and Shift tracks are independent — Left Cmd then Right Shift does not trigger — and **must both stay live on every platform** (rationale). +- Left Cmd then Right Shift does not trigger, and **both modifiers must stay live on every platform** (rationale). - **Must consume every bare Meta/Shift press outside the terminal context in this detector**, so no later handler mistakes it for a command key. -- **Must cancel a pending in-Wall mode-exit gesture when any non-Meta/Shift key intervenes.** +- **Must cancel a pending mode-exit gesture when any other key intervenes**, the other modifier included. -Source of truth: `handleDualTap` in `lib/src/components/wall/keyboard/handle-dual-tap.ts`. +Source of truth: `useWallKeyboard` in `lib/src/components/wall/use-wall-keyboard.ts`. ## Keyboard shortcuts (command mode) diff --git a/docs/specs/layout.rationale.md b/docs/specs/layout.rationale.md index c0d96bcdd..a6569b28e 100644 --- a/docs/specs/layout.rationale.md +++ b/docs/specs/layout.rationale.md @@ -54,7 +54,7 @@ xterm.js paints only its own rendered surface, and integer row fitting leaves a ## Mode switching -**Why both gesture tracks stay live everywhere.** Keyboards with no right Meta key are common on Windows and Linux laptops, so the Shift track is the only available gesture there. Keeping both live on every platform avoids a platform switch inside the detector and leaves macOS users a fallback when a hand is already on Shift. +**Why both gesture modifiers stay live everywhere.** Keyboards with no right Meta key are common on Windows and Linux laptops, so the Shift track is the only available gesture there. Keeping both live on every platform avoids a platform switch inside the detector and leaves macOS users a fallback when a hand is already on Shift. ## Split cwd inheritance diff --git a/lib/src/components/wall/keyboard/handle-dual-tap.test.ts b/lib/src/components/wall/keyboard/handle-dual-tap.test.ts index 7b956154c..5edc70427 100644 --- a/lib/src/components/wall/keyboard/handle-dual-tap.test.ts +++ b/lib/src/components/wall/keyboard/handle-dual-tap.test.ts @@ -1,11 +1,11 @@ /** * @vitest-environment jsdom */ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { handleDualTap } from './handle-dual-tap'; -import type { DualTapState, WallKeyboardCtx } from './types'; +import type { WallKeyboardCtx } from './types'; -function makeCtx(mode: 'command' | 'passthrough' = 'passthrough'): { +function makeCtx(mode: 'command' | 'passthrough'): { ctx: WallKeyboardCtx; exitTerminalMode: ReturnType; } { @@ -17,112 +17,30 @@ function makeCtx(mode: 'command' | 'passthrough' = 'passthrough'): { return { ctx, exitTerminalMode }; } -function makeState(): DualTapState { - return { - lastCmdSide: { current: null }, - lastCmdTime: { current: 0 }, - lastShiftSide: { current: null }, - lastShiftTime: { current: 0 }, - }; -} - -/** location 1 = left, 2 = right (DOM_KEY_LOCATION_{LEFT,RIGHT}). */ -function keydown(key: string, location: number): KeyboardEvent { - return new KeyboardEvent('keydown', { key, location }); +function keydown(key: string): KeyboardEvent { + return new KeyboardEvent('keydown', { key }); } describe('handleDualTap', () => { - let now = 0; - - beforeEach(() => { - now = 1000; - vi.spyOn(Date, 'now').mockImplementation(() => now); - }); - - afterEach(() => { - vi.restoreAllMocks(); - }); - - it('exits passthrough on left-then-right Meta within 500ms', () => { - const { ctx, exitTerminalMode } = makeCtx('passthrough'); - const state = makeState(); - - expect(handleDualTap(keydown('Meta', 1), ctx, state)).toBe(true); - expect(exitTerminalMode).not.toHaveBeenCalled(); - - now += 200; - expect(handleDualTap(keydown('Meta', 2), ctx, state)).toBe(true); - expect(exitTerminalMode).toHaveBeenCalledTimes(1); - // State resets after a completed gesture so the next press starts fresh. - expect(state.lastCmdSide.current).toBeNull(); - }); - - it('does not exit when the right press lands after the 500ms window', () => { - const { ctx, exitTerminalMode } = makeCtx('passthrough'); - const state = makeState(); - - handleDualTap(keydown('Meta', 1), ctx, state); - now += 500; // boundary is exclusive (< 500), so 500ms is too late - handleDualTap(keydown('Meta', 2), ctx, state); - - expect(exitTerminalMode).not.toHaveBeenCalled(); - }); - - it('does not exit on right-then-left ordering', () => { + it('exits passthrough on a completed gesture', () => { const { ctx, exitTerminalMode } = makeCtx('passthrough'); - const state = makeState(); - - handleDualTap(keydown('Meta', 2), ctx, state); - now += 100; - handleDualTap(keydown('Meta', 1), ctx, state); - + expect(handleDualTap(keydown('Meta'), ctx, false)).toBe(true); expect(exitTerminalMode).not.toHaveBeenCalled(); - }); - - it('exits passthrough on left-then-right Shift, independently of Meta', () => { - const { ctx, exitTerminalMode } = makeCtx('passthrough'); - const state = makeState(); - - expect(handleDualTap(keydown('Shift', 1), ctx, state)).toBe(true); - now += 100; - expect(handleDualTap(keydown('Shift', 2), ctx, state)).toBe(true); - + expect(handleDualTap(keydown('Meta'), ctx, true)).toBe(true); expect(exitTerminalMode).toHaveBeenCalledTimes(1); - expect(state.lastShiftSide.current).toBeNull(); - }); - - it('does not cross-trigger between Meta and Shift', () => { - const { ctx, exitTerminalMode } = makeCtx('passthrough'); - const state = makeState(); - - // Left Meta then right Shift is not a completed gesture for either track. - handleDualTap(keydown('Meta', 1), ctx, state); - now += 100; - handleDualTap(keydown('Shift', 2), ctx, state); - - expect(exitTerminalMode).not.toHaveBeenCalled(); }); it('consumes Meta and Shift but ignores other keys', () => { const { ctx } = makeCtx('passthrough'); - const state = makeState(); - - expect(handleDualTap(keydown('Meta', 1), ctx, state)).toBe(true); - expect(handleDualTap(keydown('Shift', 1), ctx, state)).toBe(true); - expect(handleDualTap(keydown('a', 0), ctx, state)).toBe(false); - expect(handleDualTap(keydown('Enter', 0), ctx, state)).toBe(false); + expect(handleDualTap(keydown('Meta'), ctx, false)).toBe(true); + expect(handleDualTap(keydown('Shift'), ctx, false)).toBe(true); + expect(handleDualTap(keydown('a'), ctx, false)).toBe(false); + expect(handleDualTap(keydown('Enter'), ctx, false)).toBe(false); }); - it('completes the gesture but does not call exit when already in command mode', () => { + it('does not call exit when already in command mode', () => { const { ctx, exitTerminalMode } = makeCtx('command'); - const state = makeState(); - - handleDualTap(keydown('Meta', 1), ctx, state); - now += 100; - handleDualTap(keydown('Meta', 2), ctx, state); - + expect(handleDualTap(keydown('Meta'), ctx, true)).toBe(true); expect(exitTerminalMode).not.toHaveBeenCalled(); - // The gesture still resets its state even when the mode guard skips exit. - expect(state.lastCmdSide.current).toBeNull(); }); }); diff --git a/lib/src/components/wall/keyboard/handle-dual-tap.ts b/lib/src/components/wall/keyboard/handle-dual-tap.ts index 8198a7d94..d2f75c4d6 100644 --- a/lib/src/components/wall/keyboard/handle-dual-tap.ts +++ b/lib/src/components/wall/keyboard/handle-dual-tap.ts @@ -1,43 +1,16 @@ -import type { DualTapState, WallKeyboardCtx } from './types'; +import type { WallKeyboardCtx } from './types'; /** - * Detects left-then-right Meta or Shift less than 500 ms apart and exits passthrough. + * Exits passthrough when `leader` (from `dualTap`) completed the gesture. * Always consumes Meta/Shift presses (returns true) so later handlers don't * misinterpret a bare modifier press as a navigation key. */ export function handleDualTap( e: KeyboardEvent, ctx: WallKeyboardCtx, - state: DualTapState, + leader: boolean, ): boolean { - if (e.key === 'Meta') { - detect(e, state.lastCmdSide, state.lastCmdTime, ctx); - return true; - } - if (e.key === 'Shift') { - detect(e, state.lastShiftSide, state.lastShiftTime, ctx); - return true; - } - return false; -} - -function detect( - e: KeyboardEvent, - lastSide: { current: 'left' | 'right' | null }, - lastTime: { current: number }, - ctx: WallKeyboardCtx, -): void { - const now = Date.now(); - const side = e.location === 1 ? 'left' : 'right'; - if ( - lastSide.current === 'left' && - side === 'right' && - now - lastTime.current < 500 - ) { - if (ctx.modeRef.current === 'passthrough') ctx.exitTerminalMode(); - lastSide.current = null; - return; - } - lastSide.current = side; - lastTime.current = now; + if (e.key !== 'Meta' && e.key !== 'Shift') return false; + if (leader && ctx.modeRef.current === 'passthrough') ctx.exitTerminalMode(); + return true; } diff --git a/lib/src/components/wall/keyboard/types.ts b/lib/src/components/wall/keyboard/types.ts index b8cf485ea..942224dd9 100644 --- a/lib/src/components/wall/keyboard/types.ts +++ b/lib/src/components/wall/keyboard/types.ts @@ -60,15 +60,6 @@ export interface WallKeyboardCtx { fireEvent: (event: WallEvent) => void; } -/** Per-press dual-tap state — left-Meta then right-Meta within 500ms exits - * passthrough mode. Same for Shift. */ -export interface DualTapState { - lastCmdSide: RefObject<'left' | 'right' | null>; - lastCmdTime: RefObject; - lastShiftSide: RefObject<'left' | 'right' | null>; - lastShiftTime: RefObject; -} - /** Last arrow direction, so the inverse arrow can return to the prior pane. */ export interface NavHistoryRef { current: { direction: string; fromId: string } | null; diff --git a/lib/src/components/wall/use-wall-keyboard.ts b/lib/src/components/wall/use-wall-keyboard.ts index 03230c059..3d744ec34 100644 --- a/lib/src/components/wall/use-wall-keyboard.ts +++ b/lib/src/components/wall/use-wall-keyboard.ts @@ -8,6 +8,7 @@ import { cancelUiSelectAll } from './keyboard/cancel-ui-select-all'; import { handlePaneShortcuts } from './keyboard/handle-pane-shortcuts'; import { handlePaneNavigation } from './keyboard/handle-pane-navigation'; import { handleWorkspaceShortcuts } from './keyboard/handle-workspace-shortcuts'; +import { dualTap, newDualTapState } from '../../lib/dual-tap'; import { isProxyOrigin } from '../../lib/iframe-proxy-registry'; import { chromeKeyboardHeld } from './chrome-keyboard-lease'; import type { NavHistoryRef, WallKeyboardCtx } from './keyboard/types'; @@ -25,10 +26,6 @@ const answeredKeys = new WeakSet(); const consumedKeys = new Set(); export function useWallKeyboard(ctx: WallKeyboardCtx): void { - const lastCmdSide = useRef<'left' | 'right' | null>(null); - const lastCmdTime = useRef(0); - const lastShiftSide = useRef<'left' | 'right' | null>(null); - const lastShiftTime = useRef(0); const navHistory = useRef(null); // Refs are stable across renders, so the handler can close over `ctx` once @@ -38,7 +35,7 @@ export function useWallKeyboard(ctx: WallKeyboardCtx): void { ctxRef.current = ctx; useEffect(() => { - const dualTapState = { lastCmdSide, lastCmdTime, lastShiftSide, lastShiftTime }; + const taps = newDualTapState(); const handler = (e: KeyboardEvent) => { const c = ctxRef.current; @@ -60,9 +57,8 @@ export function useWallKeyboard(ctx: WallKeyboardCtx): void { }; const dispatch = (e: KeyboardEvent, c: WallKeyboardCtx) => { - // Any other key cancels a pending leader: a left-Shift capital, a word, - // then a right-Shift capital is typing, not the gesture. - if (e.key !== 'Meta' && e.key !== 'Shift') lastCmdSide.current = lastShiftSide.current = null; + // Before the context branch, so its keys cancel a pending tap too. + const leader = dualTap(taps, e); const context = (e.target as HTMLElement | null)?.closest?.('[data-terminal-context]'); if (context) { if (handleContextCopy(e)) return; @@ -72,7 +68,7 @@ export function useWallKeyboard(ctx: WallKeyboardCtx): void { if (helperId) handleMouseSelectionKeys(e, { ...c, selectedIdRef: { current: helperId } }); return; } - if (handleDualTap(e, c, dualTapState)) return; + if (handleDualTap(e, c, leader)) return; // Before every mode/renaming gate below: a focused text field owns its // clipboard chords no matter what the wall is doing. if (handleEditableClipboard(e)) return; diff --git a/lib/src/host/iframe-proxy-rewrite.test.ts b/lib/src/host/iframe-proxy-rewrite.test.ts index e2d56152c..09febda73 100644 --- a/lib/src/host/iframe-proxy-rewrite.test.ts +++ b/lib/src/host/iframe-proxy-rewrite.test.ts @@ -105,6 +105,20 @@ describe('instrumentHtml', () => { expect(IFRAME_SHIM).toContain('pushState'); }); + it('posts the leader for a left-then-right tap, and not when another key falls between', () => { + const delivered: unknown[] = []; + const frame = shimFrame(APP, (data) => delivered.push(data)); + const key = (k: string, location = 0) => frame.emit('keydown', { key: k, location }); + const leaders = () => delivered.filter(d => (d as { __dormouse: string }).__dormouse === 'leader').length; + key('Shift', 1); key('Shift', 2); + key('Meta', 1); key('Meta', 2); + expect(leaders()).toBe(2); + // "I Am": typing, not the gesture. + key('Shift', 1); key('I'); key(' '); key('Shift', 2); + key('Meta', 1); key('c'); key('Meta', 2); + expect(leaders()).toBe(2); + }); + it('intercepts new-tab attempts (target=_blank / window.open) as open-window', () => { expect(IFRAME_SHIM).toContain("'open-window'"); // window.open is overridden so popups become a new pane rather than vanishing. diff --git a/lib/src/host/iframe-proxy-rewrite.ts b/lib/src/host/iframe-proxy-rewrite.ts index 8fd26b2b4..c7716f062 100644 --- a/lib/src/host/iframe-proxy-rewrite.ts +++ b/lib/src/host/iframe-proxy-rewrite.ts @@ -9,6 +9,7 @@ * runtime-agnostic. */ +import { dualTap, newDualTapState } from '../lib/dual-tap'; import { ipv4Value, isLoopbackHostname } from '../lib/ip-literal'; /** Header bag shape both `http.IncomingHttpHeaders` and a plain map satisfy. */ @@ -89,13 +90,14 @@ export function frameAncestorsCsp(embedderOrigins: string[]): string { // user-supplied, so it is not an eval vector. Injected inline into served HTML; // this is why the default drops upstream CSP whole rather than per-directive (an // inline script needs `script-src` gone as much as the frame needs -// `frame-ancestors` gone). It posts four message kinds to the Wall and nothing +// `frame-ancestors` gone). It posts five message kinds to the Wall and nothing // else (every other keystroke flows to the tool). A nested document relays the // three pane-level kinds through its same-origin proxy parents until the outer // document reaches the app; its document-level location stays inside the outer // frame: -// - `leader`: the reserved dual-tap ⌘/⇧ chord (matching handle-dual-tap.ts), -// so the global chord keeps working with the frame focused. +// - `leader`: the reserved dual-tap ⌘/⇧ chord, detected by the Wall's own +// `dualTap` embedded as source, so the global chord keeps working with the +// frame focused. // - `pointerdown`: a click landed in the frame. A cross-origin click reaches // only the frame, so the Wall can't see it; this lets it select the pane / // enter passthrough. It's genuine user input, so it can't loop with the @@ -106,6 +108,8 @@ export function frameAncestorsCsp(embedderOrigins: string[]): string { // - `open-window`: an anchor with any target but `_self` (`_blank`, `_top`, // `_parent`, a named frame) or a `window.open` the single-frame renderer // can't honor; the parent offers it as a new pane instead. +// - `theme-request`: the document loaded and wants the app's theme; it stays +// in the outer document. export function iframeShim(embedderOrigin: string): string { return `(function(){ var P=window.parent; @@ -162,16 +166,9 @@ export function iframeShim(embedderOrigin: string): string { } return null; } - function tap(s,e){ - var now=Date.now(),side=e.location===1?'left':'right'; - if(s.side==='left'&&side==='right'&&now-s.time<500){s.side=null;return true;} - s.side=side;s.time=now;return false; - } - var cmd={side:null,time:0},shift={side:null,time:0}; - addEventListener('keydown',function(e){ - if(e.key==='Meta'){if(tap(cmd,e))post('leader');} - else if(e.key==='Shift'){if(tap(shift,e))post('leader');} - },true); + // Bound by position: a minifier may rename the function itself. + var dualTap=${dualTap},taps=(${newDualTapState})(); + addEventListener('keydown',function(e){if(dualTap(taps,e))post('leader');},true); addEventListener('pointerdown',function(){post('pointerdown');},true); addEventListener('click',function(e){ var a=anchorHref(e); diff --git a/lib/src/lib/dual-tap.test.ts b/lib/src/lib/dual-tap.test.ts new file mode 100644 index 000000000..ab6348f8b --- /dev/null +++ b/lib/src/lib/dual-tap.test.ts @@ -0,0 +1,47 @@ +import { runInNewContext } from 'node:vm'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { dualTap, newDualTapState } from './dual-tap'; + +/** [key, location, ms since the previous key]; location 1 = left, 2 = right. */ +type Press = [string, number, number]; + +// The iframe shim runs the function from its source text, so it must work with +// nothing but the built-ins in scope. +const fromSource = runInNewContext(`(${dualTap})`, { Date }) as typeof dualTap; + +describe.each([ + ['dualTap', dualTap], + ['dualTap from its source', fromSource], +])('%s', (_, tap) => { + let now = 0; + beforeEach(() => { + now = 1000; + vi.spyOn(Date, 'now').mockImplementation(() => now); + }); + afterEach(() => { + vi.restoreAllMocks(); + }); + + function leaders(presses: Press[]): number { + const state = newDualTapState(); + let count = 0; + for (const [key, location, ms] of presses) { + now += ms; + if (tap(state, { key, location })) count++; + } + return count; + } + + it.each<[string, Press[], number]>([ + ['left-then-right Meta', [['Meta', 1, 0], ['Meta', 2, 200]], 1], + ['left-then-right Shift', [['Shift', 1, 0], ['Shift', 2, 100]], 1], + ['the right press at 500ms (exclusive)', [['Meta', 1, 0], ['Meta', 2, 500]], 0], + ['right-then-left', [['Meta', 2, 0], ['Meta', 1, 100]], 0], + ['left Meta then right Shift', [['Meta', 1, 0], ['Shift', 2, 100]], 0], + ['the other modifier between the taps', [['Shift', 1, 0], ['Meta', 1, 50], ['Shift', 2, 50]], 0], + ['"I Am": another key between the taps', [['Shift', 1, 0], ['I', 0, 50], [' ', 0, 50], ['Shift', 2, 50]], 0], + ['a third right press after a completed gesture', [['Meta', 1, 0], ['Meta', 2, 100], ['Meta', 2, 100]], 1], + ])('%s → %i leader', (_, presses, want) => { + expect(leaders(presses)).toBe(want); + }); +}); diff --git a/lib/src/lib/dual-tap.ts b/lib/src/lib/dual-tap.ts new file mode 100644 index 000000000..3064a9bf9 --- /dev/null +++ b/lib/src/lib/dual-tap.ts @@ -0,0 +1,30 @@ +/** The last keydown, when it was a left Meta or Shift that can start the gesture. */ +export interface DualTapState { + left: { key: string; time: number } | null; +} + +export function newDualTapState(): DualTapState { + return { left: null }; +} + +/** + * Feed every keydown; true when it completes the leader gesture: left-then-right + * Meta, or Shift, less than 500 ms apart. Any other key in between, the other + * modifier included, cancels it: a left-Shift capital, a word, then a + * right-Shift capital is typing. + * + * The iframe shim embeds this function's source (iframe-proxy-rewrite.ts), so it + * must stay self-contained: no imports, no module-scope references, no syntax a + * bundler lowers to a helper, no inner named functions. + */ +export function dualTap(s: DualTapState, e: { key: string; location: number }): boolean { + const left = s.left; + s.left = null; + if (e.key !== 'Meta' && e.key !== 'Shift') return false; + const now = Date.now(); + if (e.location === 1) { + s.left = { key: e.key, time: now }; + return false; + } + return left !== null && left.key === e.key && now - left.time < 500; +} diff --git a/lib/src/stories/Wall.stories.tsx b/lib/src/stories/Wall.stories.tsx index 2e15cd961..591b828f7 100644 --- a/lib/src/stories/Wall.stories.tsx +++ b/lib/src/stories/Wall.stories.tsx @@ -47,7 +47,7 @@ async function expectCount(count: () => number, want: number, what: string) { /** * Return to command mode with the leader chord — left Shift then right Shift - * within 500ms (`keyboard/handle-dual-tap.ts`). + * within 500ms (`lib/dual-tap.ts`). * * Required between splits: a split is an intent to use the new terminal, so * `addSplitPanel` enters passthrough, and the wall ignores pane shortcuts there.