Skip to content

Commit fd1831b

Browse files
r4ijur4iju
committed
fix(web): scope the paste dedupe to the shortcut's own gesture
The native paste can arrive after the clipboard read and after other keys move, so clearing the record on any keydown or keyup let the duplicate through again. Only the shortcut key's own keyup, a new paste shortcut, a blur, or the matching paste now ends the gesture. Blur covers releases the terminal never sees, which previously left a later Edit > Paste of the same text dropped.
1 parent 10a8bf9 commit fd1831b

2 files changed

Lines changed: 60 additions & 26 deletions

File tree

‎apps/web/src/terminal/ghostty/surface.test.ts‎

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,22 @@ describe("GhosttyTerminalSurface visibility", () => {
203203
}),
204204
);
205205
},
206+
blur() {
207+
input?.dispatchEvent(new Event("blur"));
208+
},
209+
/**
210+
* Serves `text` to the paste shortcut's clipboard reads. The returned
211+
* function asserts how many reads started and waits for the last one to
212+
* settle, so no read can land after the caller's next assertion.
213+
*/
214+
stubClipboard(text: string) {
215+
const readText = vi.fn(async () => text);
216+
vi.stubGlobal("navigator", { platform: "MacIntel", clipboard: { readText } });
217+
return async (reads: number) => {
218+
expect(readText).toHaveBeenCalledTimes(reads);
219+
await readText.mock.results[reads - 1]?.value;
220+
};
221+
},
206222
paste(text: string) {
207223
input?.dispatchEvent(
208224
Object.assign(new Event("paste", { cancelable: true }), {
@@ -338,12 +354,11 @@ describe("GhosttyTerminalSurface visibility", () => {
338354

339355
it("pastes once when the shortcut's clipboard read lands before the native paste", async () => {
340356
const harness = createHarness();
341-
const readText = vi.fn(async () => "echo hi");
342-
vi.stubGlobal("navigator", { platform: "MacIntel", clipboard: { readText } });
357+
const settleReads = harness.stubClipboard("echo hi");
343358
await harness.create({ beforeKey: () => true });
344359

345360
harness.key("v", { metaKey: true });
346-
await readText.mock.results[0]?.value;
361+
await settleReads(1);
347362
expect(harness.onData).toHaveBeenCalledTimes(1);
348363
harness.paste("echo hi");
349364
harness.key("v", { metaKey: true }, "keyup");
@@ -353,30 +368,52 @@ describe("GhosttyTerminalSurface visibility", () => {
353368
// its read must not land on top of the native paste that won.
354369
harness.key("v", { metaKey: true });
355370
harness.paste("echo hi");
356-
await readText.mock.results[1]?.value;
371+
await settleReads(2);
357372
expect(harness.onData).toHaveBeenCalledTimes(2);
358373
});
359374

375+
it("still drops the late native paste when other keys move mid-gesture", async () => {
376+
const harness = createHarness();
377+
const settleReads = harness.stubClipboard("echo hi");
378+
await harness.create({ beforeKey: () => true });
379+
380+
harness.key("v", { metaKey: true });
381+
await settleReads(1);
382+
// Rolling typing: another key goes down and a previous one comes up
383+
// before the browser's paste event arrives.
384+
harness.key("Shift", { shiftKey: true });
385+
harness.key("Shift", {}, "keyup");
386+
harness.paste("echo hi");
387+
388+
expect(harness.onData.mock.calls.filter(([data]) => data === "echo hi")).toHaveLength(1);
389+
});
390+
360391
it("keeps a later menu paste once the shortcut that only the read served ends", async () => {
361392
const harness = createHarness();
362-
const readText = vi.fn(async () => "echo hi");
363-
vi.stubGlobal("navigator", { platform: "MacIntel", clipboard: { readText } });
393+
const settleReads = harness.stubClipboard("echo hi");
364394
await harness.create({ beforeKey: () => true });
365395

366396
// No native paste for the shortcut: the read is the only delivery. An
367397
// Edit → Paste of the same text afterwards has no keydown of its own.
368398
harness.key("v", { metaKey: true });
369-
await readText.mock.results[0]?.value;
399+
await settleReads(1);
370400
harness.key("v", { metaKey: true }, "keyup");
371401
harness.paste("echo hi");
372402
expect(harness.onData).toHaveBeenCalledTimes(2);
373403

374404
// Same when the read only settles after the shortcut was released.
375405
harness.key("v", { metaKey: true });
376406
harness.key("v", { metaKey: true }, "keyup");
377-
await readText.mock.results[1]?.value;
407+
await settleReads(2);
378408
harness.paste("echo hi");
379409
expect(harness.onData).toHaveBeenCalledTimes(4);
410+
411+
// Same when focus leaves before the release, so the terminal never sees it.
412+
harness.key("v", { metaKey: true });
413+
await settleReads(3);
414+
harness.blur();
415+
harness.paste("echo hi");
416+
expect(harness.onData).toHaveBeenCalledTimes(6);
380417
});
381418

382419
it("starts a selection when dragging from a link", async () => {

‎apps/web/src/terminal/ghostty/surface.ts‎

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -625,10 +625,12 @@ export class GhosttyTerminalSurface {
625625
private theme: GhosttyTheme;
626626
private readonly suppressedKeyCodes = new Set<string>();
627627
private pasteShortcutToken = 0;
628-
/** Whether the paste shortcut is still held, so its native paste can still follow. */
629-
private pasteShortcutHeld = false;
630-
/** Text the held paste shortcut's clipboard read already sent. */
631-
private shortcutPasteReadText: string | null = null;
628+
// The paste shortcut gesture whose native paste may still arrive, and the
629+
// text its clipboard read already sent (empty until then; onPaste ignores
630+
// empty data). The browser may deliver that paste after the read and after
631+
// other keys move, but before the shortcut key's own keyup, so only that
632+
// keyup, a new shortcut, or blur ends the gesture.
633+
private pasteShortcutGesture: { code: string; readText: string } | null = null;
632634
private copyShortcutToken = 0;
633635
private clearSelectionAfterCopy = false;
634636
private primedCopySelection = "";
@@ -1057,8 +1059,6 @@ export class GhosttyTerminalSurface {
10571059
}
10581060

10591061
private readonly onKeyDown = (event: KeyboardEvent) => {
1060-
this.pasteShortcutHeld = false;
1061-
this.shortcutPasteReadText = null;
10621062
// Presses handled outside the terminal must also swallow their release:
10631063
// beforeKey runs side effects (keybindings, navigation sends), so it cannot
10641064
// be consulted again on keyup, and Kitty report-event-types sessions would
@@ -1124,23 +1124,22 @@ export class GhosttyTerminalSurface {
11241124
}
11251125
if (isTerminalPasteShortcut(event)) {
11261126
this.suppressedKeyCodes.add(event.code);
1127-
this.pasteShortcutHeld = true;
1127+
const gesture = { code: event.code, readText: "" };
1128+
this.pasteShortcutGesture = gesture;
11281129
const clipboard = navigator.clipboard;
11291130
if (typeof clipboard?.readText === "function") {
11301131
// Race the async clipboard read against the browser's own paste event;
11311132
// the read covers browsers whose paste shortcut produces no paste
11321133
// event. Not preventing the default keeps the native path alive when
1133-
// the read is denied. Either side can land first: a native paste
1134-
// claims the token, and a read delivered while the shortcut is held
1135-
// records its text so the native paste that follows it is dropped in
1136-
// onPaste. The keyboard paste fires before keyup, which ends the window.
1134+
// the read is denied. A native paste that lands first claims the
1135+
// token; a read that lands first records its text on the gesture.
11371136
const token = ++this.pasteShortcutToken;
11381137
void clipboard.readText().then(
11391138
(text) => {
11401139
if (this.disposed || this.pasteShortcutToken !== token) return;
11411140
this.pasteShortcutToken += 1;
11421141
if (text.length === 0) return;
1143-
if (this.pasteShortcutHeld) this.shortcutPasteReadText = text;
1142+
if (this.pasteShortcutGesture === gesture) gesture.readText = text;
11441143
this.options.onData(this.core.encodePaste(text));
11451144
},
11461145
() => {
@@ -1166,10 +1165,7 @@ export class GhosttyTerminalSurface {
11661165
};
11671166

11681167
private readonly onKeyUp = (event: KeyboardEvent) => {
1169-
// Any release ends the paste gesture: macOS never delivers V's keyup while
1170-
// Cmd is held, so the Cmd release is often the only one.
1171-
this.pasteShortcutHeld = false;
1172-
this.shortcutPasteReadText = null;
1168+
if (event.code === this.pasteShortcutGesture?.code) this.pasteShortcutGesture = null;
11731169
if (this.suppressedKeyCodes.delete(event.code)) return;
11741170
if (isTerminalCompositionKey(event, this.composing)) {
11751171
return;
@@ -1191,6 +1187,7 @@ export class GhosttyTerminalSurface {
11911187

11921188
private readonly onBlur = () => {
11931189
this.focused = false;
1190+
this.pasteShortcutGesture = null;
11941191
this.refreshHoveredLink();
11951192
// Suppressions survive blur deliberately: a shortcut that moves focus (for
11961193
// example terminal-toggle) must still swallow its own keyup if focus comes
@@ -1250,8 +1247,8 @@ export class GhosttyTerminalSurface {
12501247
event.preventDefault();
12511248
const data = event.clipboardData?.getData("text/plain") ?? "";
12521249
if (data.length === 0) return;
1253-
if (data === this.shortcutPasteReadText) {
1254-
this.shortcutPasteReadText = null;
1250+
if (data === this.pasteShortcutGesture?.readText) {
1251+
this.pasteShortcutGesture = null;
12551252
return;
12561253
}
12571254
// The native paste won the race with actual text; a pending clipboard read

0 commit comments

Comments
 (0)