mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 10:39:32 +01:00
fix(paste): cancel the Ctrl+V keydown once the paste trap has the clipboard
Ctrl+V in Firefox still pasted the clipboard twice some of the time.
Right-click → Paste never did.
`_handleImagePaste()` focuses a hidden contenteditable trap, calls
`document.execCommand('paste')`, and reads the clipboard from the paste
event that lands on the trap. Firefox dispatches that event synchronously,
inside the `execCommand` call. It then delivers the key's own paste as a
separate task, about 10 ms later in Playwright's Firefox. The one-shot flag
from b87bc687 drops that second event, but only if it still reaches the
trap. The trap removes itself and refocuses the terminal on a
`setTimeout(0)` queued by the first event, and that timer races the second
paste. When the timer wins, the second paste lands on xterm's textarea,
xterm's own `handlePasteEvent` sends the clipboard again, and the flag
never sees it.
The trap now cancels the keydown when its paste listener has already run
by the time `execCommand` returns, so Firefox never issues the second
paste. Both callers pass the keydown as `target.keyEvent`: the main pane's
key handler in terminal-ui.js and a tile's in terminal-tile.js. Chromium
and WebKit refuse the command and dispatch nothing from it, so their
keydown is left alone, because its default action delivers their only
paste event.
Measured in Playwright's Firefox with the trap removed before the key's
paste arrived: the paste reached xterm's textarea and the text went in
twice. With the keydown cancelled, the key's paste never fired. End to end
on an isolated instance with a shell session, 20 Ctrl+V presses gave 20
pastes in both Firefox and Chromium.
test/image-paste-trap.test.ts gains a harness option in which
`execCommand` delivers the paste from inside the call, as Firefox does.
New cases cover the keydown being cancelled for the main pane and for a
tile, being left alone when `execCommand` delivers nothing, and a caller
that passes no keydown. The two cancel cases fail without this change.
test/terminal-tile-input.test.ts now expects the tile to pass its keydown
along with its terminal and session.
Docs: the Terminal paste section of docs/architecture-invariants.md and
the paste-trap entry in src/web/CLAUDE.md now describe the race and the
rule. The section's old claim that the key's default action alone always
reaches the trap is replaced, since in Firefox it can miss it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
f62c1f7e2f
commit
6c5d53ebfe
7 files changed
+126
-26
No files matched your search
@@ -7,9 +7,11 @@
|
||||
* Ctrl+V: the `document.execCommand('paste')` the function issues itself, and
|
||||
* the keydown's own default action, which still runs because xterm's custom key
|
||||
* handler returns false without cancelling the event. A browser that honours
|
||||
* execCommand('paste') therefore fires the trap's listener twice, and the
|
||||
* execCommand('paste') (Firefox) therefore produced two paste events, and the
|
||||
* clipboard text used to reach the PTY twice with it — while right-click →
|
||||
* Paste, which involves no keydown, stayed correct.
|
||||
* Paste, which involves no keydown, stayed correct. Firefox delivers the key's
|
||||
* paste as a later task, so it can also miss the trap and reach xterm's
|
||||
* textarea; the trap therefore cancels the keydown once execCommand delivered.
|
||||
*
|
||||
* Loads the browser module into a vm sandbox with a fake document, so the tests
|
||||
* drive the trap's listener directly rather than through a real browser.
|
||||
@@ -64,6 +66,13 @@ interface Harness {
|
||||
/** How many trap divs are still attached to the fake body. */
|
||||
attachedTraps: () => number;
|
||||
runTimers: () => void;
|
||||
/** How many times the Ctrl+V keydown handed to _handleImagePaste() was cancelled. */
|
||||
keydownCancels: () => number;
|
||||
}
|
||||
|
||||
interface PastePayload {
|
||||
text?: string;
|
||||
images?: string[];
|
||||
}
|
||||
|
||||
interface PasteHarnessOptions {
|
||||
@@ -73,6 +82,14 @@ interface PasteHarnessOptions {
|
||||
localEchoPending?: string;
|
||||
/** Terminal the Ctrl+V came from, when it is not the primary pane. */
|
||||
terminal?: TargetTerminal;
|
||||
/**
|
||||
* What `document.execCommand('paste')` delivers to the trap before it
|
||||
* returns. Firefox dispatches a real paste event from inside the call;
|
||||
* Chromium and WebKit refuse the command and dispatch nothing (the default).
|
||||
*/
|
||||
execCommandDelivers?: PastePayload;
|
||||
/** Call _handleImagePaste() without the keydown, as an older caller would. */
|
||||
omitKeyEvent?: boolean;
|
||||
}
|
||||
|
||||
function loadPasteHarness(options: PasteHarnessOptions = {}): Harness {
|
||||
@@ -81,6 +98,19 @@ function loadPasteHarness(options: PasteHarnessOptions = {}): Harness {
|
||||
const attached = new Set<FakeTrap>();
|
||||
const timers: Array<() => void> = [];
|
||||
|
||||
const firePaste = ({ text = '', images = [] }: PastePayload) => {
|
||||
const items = images.map((type) => ({ type, getAsFile: () => ({ type }) }));
|
||||
const event = {
|
||||
clipboardData: {
|
||||
items,
|
||||
getData: () => text,
|
||||
},
|
||||
preventDefault: () => {},
|
||||
stopPropagation: () => {},
|
||||
};
|
||||
for (const fn of listeners) fn(event);
|
||||
};
|
||||
|
||||
const documentObj = {
|
||||
createElement: (): FakeTrap => {
|
||||
const trap: FakeTrap = {
|
||||
@@ -106,8 +136,12 @@ function loadPasteHarness(options: PasteHarnessOptions = {}): Harness {
|
||||
},
|
||||
},
|
||||
// A browser that honours the command fires the trap's paste listener from
|
||||
// here as well; the tests model that by firing the listener twice.
|
||||
execCommand: () => true,
|
||||
// inside this call. Firefox then returns false, because the trap cancels
|
||||
// the event and the command never completes.
|
||||
execCommand: () => {
|
||||
if (options.execCommandDelivers) firePaste(options.execCommandDelivers);
|
||||
return false;
|
||||
},
|
||||
getElementById: () => null,
|
||||
};
|
||||
|
||||
@@ -184,25 +218,24 @@ function loadPasteHarness(options: PasteHarnessOptions = {}): Harness {
|
||||
const toasts: string[] = [];
|
||||
app.showToast = (message: string) => toasts.push(message);
|
||||
|
||||
const target =
|
||||
let keydownCancels = 0;
|
||||
const keyEvent = {
|
||||
type: 'keydown',
|
||||
key: 'v',
|
||||
ctrlKey: true,
|
||||
preventDefault: () => {
|
||||
keydownCancels += 1;
|
||||
},
|
||||
};
|
||||
const target: Record<string, unknown> =
|
||||
options.sessionId || options.terminal
|
||||
? { terminal: options.terminal ?? app.terminal, sessionId: options.sessionId || 'session-1' }
|
||||
: undefined;
|
||||
: {};
|
||||
if (!options.omitKeyEvent) target.keyEvent = keyEvent;
|
||||
(app._handleImagePaste as (t?: object) => void).call(app, target);
|
||||
|
||||
return {
|
||||
firePaste({ text = '', images = [] }) {
|
||||
const items = images.map((type) => ({ type, getAsFile: () => ({ type }) }));
|
||||
const event = {
|
||||
clipboardData: {
|
||||
items,
|
||||
getData: () => text,
|
||||
},
|
||||
preventDefault: () => {},
|
||||
stopPropagation: () => {},
|
||||
};
|
||||
for (const fn of listeners) fn(event);
|
||||
},
|
||||
firePaste,
|
||||
pastedText,
|
||||
inputted,
|
||||
sentInputs,
|
||||
@@ -215,6 +248,7 @@ function loadPasteHarness(options: PasteHarnessOptions = {}): Harness {
|
||||
const pending = timers.splice(0, timers.length);
|
||||
for (const fn of pending) fn();
|
||||
},
|
||||
keydownCancels: () => keydownCancels,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -378,6 +412,49 @@ describe('Ctrl+V paste trap', () => {
|
||||
h.runTimers();
|
||||
expect(h.attachedTraps()).toBe(0);
|
||||
});
|
||||
|
||||
// Firefox delivers the execCommand paste inside the call and the key's own
|
||||
// paste as a later task. When the trap's cleanup timer runs first, that
|
||||
// later paste lands on xterm's textarea instead of the trap, and xterm
|
||||
// pastes it a second time. Cancelling the keydown is what stops it.
|
||||
it('cancels the keydown when execCommand already delivered the paste', () => {
|
||||
const h = loadPasteHarness({ execCommandDelivers: { text: 'hello world' } });
|
||||
|
||||
expect(h.inputted).toEqual(['\x1b[200~hello world\x1b[201~']);
|
||||
expect(h.keydownCancels()).toBe(1);
|
||||
});
|
||||
|
||||
it('cancels the split-pane keydown the same way', () => {
|
||||
const paneInputs: string[] = [];
|
||||
const pane: TargetTerminal = { input: (data) => paneInputs.push(data), focus: () => {} };
|
||||
const h = loadPasteHarness({
|
||||
sessionId: 'pane-b',
|
||||
terminal: pane,
|
||||
execCommandDelivers: { text: 'into pane b' },
|
||||
});
|
||||
|
||||
expect(paneInputs).toEqual(['\x1b[200~into pane b\x1b[201~']);
|
||||
expect(h.keydownCancels()).toBe(1);
|
||||
});
|
||||
|
||||
it('leaves the keydown alone when execCommand delivered nothing', () => {
|
||||
// Chromium and WebKit: the key's default action is the only route into
|
||||
// the trap, so cancelling it would lose the paste entirely.
|
||||
const h = loadPasteHarness();
|
||||
expect(h.keydownCancels()).toBe(0);
|
||||
|
||||
h.firePaste({ text: 'hello world' });
|
||||
|
||||
expect(h.inputted).toEqual(['\x1b[200~hello world\x1b[201~']);
|
||||
expect(h.keydownCancels()).toBe(0);
|
||||
});
|
||||
|
||||
it('still pastes once when called without the keydown', () => {
|
||||
const h = loadPasteHarness({ execCommandDelivers: { text: 'hello world' }, omitKeyEvent: true });
|
||||
h.firePaste({ text: 'hello world' });
|
||||
|
||||
expect(h.inputted).toEqual(['\x1b[200~hello world\x1b[201~']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('image upload insertion policy', () => {
|
||||
|
||||
@@ -741,10 +741,13 @@ describe('TerminalTile links and paste follow THIS pane', () => {
|
||||
app._handleImagePaste = paste;
|
||||
const { term } = await connectTile(app);
|
||||
|
||||
const handled = term.keyHandler!({ type: 'keydown', key: 'v', ctrlKey: true, code: 'KeyV' });
|
||||
const keyEvent = { type: 'keydown', key: 'v', ctrlKey: true, code: 'KeyV' };
|
||||
const handled = term.keyHandler!(keyEvent);
|
||||
|
||||
expect(handled).toBe(false);
|
||||
expect(paste).toHaveBeenCalledWith({ terminal: term, sessionId: 's-tile' });
|
||||
// The keydown rides along so the trap can cancel it once execCommand('paste')
|
||||
// has delivered (Firefox), instead of letting a second paste reach xterm.
|
||||
expect(paste).toHaveBeenCalledWith({ terminal: term, sessionId: 's-tile', keyEvent });
|
||||
});
|
||||
|
||||
it('leaves Ctrl+Shift+V (voice input) out of the paste trap', async () => {
|
||||
|
||||
Reference in new issue
Block a user