authorgravatar for maria@kuuro.netmaria <maria@kuuro.net> 2026-09-04 21:10:18-04:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-04 21:10:18-04:00
logb5fb3fba0fb3dbd1bc2e29886232321cc06863d5
treee8fecb44c22c7b622671fa3be5fb681be5feb75f
parent2dca7a1eddd27c823e4d9d0a316864c4ad9f95c5
signature Signed by PGP key B5690EEEBB952194

fix(desktop): isolate preview keyboard shortcuts from the host (#9840)

2 files changed, 87 insertions(+), 47 deletions(-)

apps/desktop/src/preview/Manager.test.ts+83
......@@ -272,6 +272,7 @@ const makeTestPreviewWebContents = (
272272 ipc: { on: vi.fn(), off: vi.fn() },
273273 send: webviewSend,
274274 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
275 setIgnoreMenuShortcuts: vi.fn(),
275276 setWindowOpenHandler: vi.fn(),
276277 debugger: {
277278 isAttached: () => false,
......@@ -382,6 +383,7 @@ const makeFaviconWebContents = (options?: {
382383 send: webviewSend,
383384 session: { fetch },
384385 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
386 setIgnoreMenuShortcuts: vi.fn(),
385387 setWindowOpenHandler: vi.fn(),
386388 executeJavaScriptInIsolatedWorld,
387389 debugger: {
......@@ -474,6 +476,67 @@ describe("PreviewManager", () => {
474476 webviewSend.mockClear();
475477 });
476478
479 effectIt.effect("keeps preview shortcuts out of the host window", () =>
480 withManager((manager) =>
481 Effect.gen(function* () {
482 const preview = makeFaviconWebContents();
483 const sendInputEvent = vi.fn();
484 const hostWebContents = { sendInputEvent };
485 Object.assign(preview.webContents, { hostWebContents });
486 fromId.mockReturnValue(preview.webContents);
487 yield* manager.setMainWindow({
488 isDestroyed: () => false,
489 once: vi.fn(),
490 webContents: hostWebContents,
491 } as never);
492 yield* manager.createTab("tab_keys");
493 yield* manager.registerWebview("tab_keys", 42);
494
495 expect(
496 (preview.webContents as Electron.WebContents).setIgnoreMenuShortcuts,
497 ).toHaveBeenCalledWith(true);
498 const beforeInput = preview.listeners.get("before-input-event")!;
499 for (const control of [false, true]) {
500 for (const key of ["k", ",", "w", "j", "q", "+", "a", "c", "v", "x"]) {
501 for (const type of ["keyDown", "keyUp"]) {
502 const preventDefault = vi.fn();
503 beforeInput(
504 { preventDefault } as never,
505 { type, key, meta: !control, control, shift: key === "j", alt: false } as never,
506 );
507 yield* Effect.yieldNow;
508 expect(preventDefault).not.toHaveBeenCalled();
509 }
510 }
511 }
512 expect(sendInputEvent).not.toHaveBeenCalled();
513
514 const preventDefault = vi.fn();
515 beforeInput(
516 { preventDefault } as never,
517 {
518 type: "keyDown",
519 key: "r",
520 meta: true,
521 control: false,
522 shift: false,
523 alt: false,
524 } as never,
525 );
526 yield* Effect.yieldNow;
527 expect(preventDefault).toHaveBeenCalledOnce();
528 expect(preview.reload).toHaveBeenCalledOnce();
529 expect(sendInputEvent).not.toHaveBeenCalled();
530
531 const setIgnoreMenuShortcuts = vi.fn();
532 preview.listeners.get("did-create-window")!({
533 webContents: { setIgnoreMenuShortcuts, setWindowOpenHandler: vi.fn() },
534 } as never);
535 expect(setIgnoreMenuShortcuts).toHaveBeenCalledWith(true);
536 }),
537 ),
538 );
539
477540 effectIt.effect("reports an unregistered webview as temporarily unavailable", () =>
478541 withManager((manager) =>
479542 Effect.gen(function* () {
......@@ -617,6 +680,7 @@ describe("PreviewManager", () => {
617680 ipc: { on: vi.fn(), off: vi.fn() },
618681 send: webviewSend,
619682 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
683 setIgnoreMenuShortcuts: vi.fn(),
620684 setWindowOpenHandler: vi.fn(),
621685 debugger: {
622686 isAttached: () => false,
......@@ -718,6 +782,7 @@ describe("PreviewManager", () => {
718782 ipc: { on: vi.fn(), off: vi.fn() },
719783 send: webviewSend,
720784 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
785 setIgnoreMenuShortcuts: vi.fn(),
721786 setWindowOpenHandler: vi.fn(),
722787 get debugger() {
723788 if (destroyed) throw new Error("Object has been destroyed");
......@@ -1222,6 +1287,7 @@ describe("PreviewManager", () => {
12221287 ipc: { on: vi.fn(), off: vi.fn() },
12231288 send: webviewSend,
12241289 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
1290 setIgnoreMenuShortcuts: vi.fn(),
12251291 setWindowOpenHandler: vi.fn(),
12261292 debugger: {
12271293 isAttached: () => false,
......@@ -1286,6 +1352,7 @@ describe("PreviewManager", () => {
12861352 ipc: { on: vi.fn(), off: vi.fn() },
12871353 send: webviewSend,
12881354 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
1355 setIgnoreMenuShortcuts: vi.fn(),
12891356 setWindowOpenHandler: vi.fn(),
12901357 debugger: {
12911358 isAttached: () => false,
......@@ -1326,6 +1393,7 @@ describe("PreviewManager", () => {
13261393 ipc: { on: vi.fn(), off: vi.fn() },
13271394 send: webviewSend,
13281395 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
1396 setIgnoreMenuShortcuts: vi.fn(),
13291397 setWindowOpenHandler: vi.fn(),
13301398 debugger: {
13311399 isAttached: () => false,
......@@ -1372,6 +1440,7 @@ describe("PreviewManager", () => {
13721440 ipc: { on: vi.fn(), off: vi.fn() },
13731441 send: webviewSend,
13741442 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
1443 setIgnoreMenuShortcuts: vi.fn(),
13751444 setWindowOpenHandler: vi.fn(),
13761445 debugger: {
13771446 isAttached: () => false,
......@@ -1427,6 +1496,7 @@ describe("PreviewManager", () => {
14271496 ipc: { on: vi.fn(), off: vi.fn() },
14281497 send: webviewSend,
14291498 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
1499 setIgnoreMenuShortcuts: vi.fn(),
14301500 setWindowOpenHandler: vi.fn(),
14311501 debugger: {
14321502 isAttached: () => false,
......@@ -1524,6 +1594,7 @@ describe("PreviewManager", () => {
15241594 ipc: { on: vi.fn(), off: vi.fn() },
15251595 send: webviewSend,
15261596 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
1597 setIgnoreMenuShortcuts: vi.fn(),
15271598 setWindowOpenHandler: vi.fn(),
15281599 debugger: {
15291600 isAttached: () => false,
......@@ -1859,6 +1930,7 @@ describe("PreviewManager", () => {
18591930 ipc: { on: vi.fn(), off: vi.fn() },
18601931 send: webviewSend,
18611932 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
1933 setIgnoreMenuShortcuts: vi.fn(),
18621934 setWindowOpenHandler: vi.fn(),
18631935 debugger: {
18641936 isAttached: () => false,
......@@ -1951,6 +2023,7 @@ describe("PreviewManager", () => {
19512023 ipc: { on: vi.fn(), off: vi.fn() },
19522024 send: webviewSend,
19532025 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
2026 setIgnoreMenuShortcuts: vi.fn(),
19542027 setWindowOpenHandler: vi.fn(),
19552028 debugger: {
19562029 isAttached: () => false,
......@@ -2368,6 +2441,7 @@ describe("PreviewManager", () => {
23682441 ipc: { on: vi.fn(), off: vi.fn() },
23692442 send: webviewSend,
23702443 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
2444 setIgnoreMenuShortcuts: vi.fn(),
23712445 setWindowOpenHandler: vi.fn(),
23722446 debugger: {
23732447 isAttached: () => false,
......@@ -2670,6 +2744,7 @@ describe("PreviewManager", () => {
26702744 ipc: { on: vi.fn(), off: vi.fn() },
26712745 send: webviewSend,
26722746 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
2747 setIgnoreMenuShortcuts: vi.fn(),
26732748 setWindowOpenHandler: vi.fn(),
26742749 debugger: {
26752750 isAttached: () => false,
......@@ -3173,6 +3248,7 @@ describe("PreviewManager", () => {
31733248 ipc: { on: vi.fn(), off: vi.fn(), removeListener: vi.fn() },
31743249 send: webviewSend,
31753250 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
3251 setIgnoreMenuShortcuts: vi.fn(),
31763252 setWindowOpenHandler: vi.fn(),
31773253 debugger: {
31783254 isAttached: () => false,
......@@ -3228,6 +3304,7 @@ describe("PreviewManager", () => {
32283304 },
32293305 send: webviewSend,
32303306 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
3307 setIgnoreMenuShortcuts: vi.fn(),
32313308 setWindowOpenHandler: vi.fn(),
32323309 debugger: {
32333310 isAttached: () => false,
......@@ -3304,6 +3381,7 @@ describe("PreviewManager", () => {
33043381 },
33053382 send: webviewSend,
33063383 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
3384 setIgnoreMenuShortcuts: vi.fn(),
33073385 setWindowOpenHandler: vi.fn(),
33083386 debugger: {
33093387 isAttached: () => false,
......@@ -3389,6 +3467,7 @@ describe("PreviewManager", () => {
33893467 goBack,
33903468 goForward,
33913469 },
3470 setIgnoreMenuShortcuts: vi.fn(),
33923471 setWindowOpenHandler: vi.fn(),
33933472 debugger: {
33943473 isAttached: () => false,
......@@ -3518,6 +3597,7 @@ describe("PreviewManager", () => {
35183597 },
35193598 send: webviewSend,
35203599 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
3600 setIgnoreMenuShortcuts: vi.fn(),
35213601 setWindowOpenHandler: vi.fn(),
35223602 debugger: {
35233603 isAttached: () => false,
......@@ -3618,6 +3698,7 @@ describe("PreviewManager", () => {
36183698 },
36193699 send: webviewSend,
36203700 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
3701 setIgnoreMenuShortcuts: vi.fn(),
36213702 setWindowOpenHandler: vi.fn(),
36223703 debugger: {
36233704 isAttached: () => false,
......@@ -3773,6 +3854,7 @@ describe("PreviewManager", () => {
37733854 },
37743855 send: webviewSend,
37753856 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
3857 setIgnoreMenuShortcuts: vi.fn(),
37763858 setWindowOpenHandler: vi.fn(),
37773859 debugger: {
37783860 isAttached: () => false,
......@@ -3837,6 +3919,7 @@ describe("PreviewManager", () => {
38373919 ipc: { on: vi.fn(), off: vi.fn() },
38383920 send: webviewSend,
38393921 navigationHistory: { canGoBack: () => false, canGoForward: () => false },
3922 setIgnoreMenuShortcuts: vi.fn(),
38403923 setWindowOpenHandler: vi.fn(),
38413924 debugger: {
38423925 isAttached: () => false,
apps/desktop/src/preview/Manager.ts+4-47
......@@ -465,22 +465,6 @@ interface ExpectedAgentInput {
465465 readonly expiresAt: number;
466466}
467467
468const APP_FORWARDED_SHORTCUTS: ReadonlyArray<{
469 key: string;
470 meta: boolean;
471 shift: boolean;
472 control: boolean;
473}> = Object.freeze([
474 // mod+shift+J → preview.toggle
475 { key: "j", meta: true, shift: true, control: false },
476 // mod+K → command palette
477 { key: "k", meta: true, shift: false, control: false },
478 // mod+, → settings (macOS convention)
479 { key: ",", meta: true, shift: false, control: false },
480 // mod+W → close tab/panel
481 { key: "w", meta: true, shift: false, control: false },
482]);
483
484468/**
485469 * Protocols a preview page may open in a real popup window.
486470 *
......@@ -1535,16 +1519,6 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
15351519 }
15361520 });
15371521
1538 const isAppShortcut = (input: Electron.Input): boolean =>
1539 input.type === "keyDown" &&
1540 APP_FORWARDED_SHORTCUTS.some(
1541 (shortcut) =>
1542 shortcut.key.toLowerCase() === input.key.toLowerCase() &&
1543 shortcut.meta === input.meta &&
1544 shortcut.shift === input.shift &&
1545 shortcut.control === input.control,
1546 );
1547
15481522 const computeNavStatus = (wc: Electron.WebContents): PreviewNavStatus => {
15491523 const url = wc.getURL();
15501524 const title = wc.getTitle();
......@@ -1819,30 +1793,11 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
18191793 }).pipe(Effect.ignore),
18201794 );
18211795 };
1822 const forwardShortcut = Effect.fn("PreviewManager.forwardShortcut")(function* (
1823 event: Electron.Event,
1824 input: Electron.Input,
1825 ) {
1826 const mainWindow = yield* Ref.get(mainWindowRef);
1827 if (!isAppShortcut(input) || Option.isNone(mainWindow) || mainWindow.value.isDestroyed()) {
1828 return;
1829 }
1830 event.preventDefault();
1831 mainWindow.value.webContents.sendInputEvent({
1832 type: "keyDown",
1833 keyCode: input.key,
1834 modifiers: [
1835 ...(input.meta ? (["meta"] as const) : []),
1836 ...(input.shift ? (["shift"] as const) : []),
1837 ...(input.control ? (["control"] as const) : []),
1838 ...(input.alt ? (["alt"] as const) : []),
1839 ],
1840 });
1841 });
18421796 // A popup opens with Electron's default handler, so the page inside it could
18431797 // otherwise spawn native windows without limit. Nothing in an OAuth flow
18441798 // opens a second popup, so the chain stops at the first one.
18451799 const windowCreated = (window: Electron.BrowserWindow): void => {
1800 window.webContents.setIgnoreMenuShortcuts(true);
18461801 window.webContents.setWindowOpenHandler(() => ({ action: "deny" }));
18471802 };
18481803 const beforeInput = (event: Electron.Event, input: Electron.Input): void => {
......@@ -1855,7 +1810,6 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
18551810 );
18561811 return;
18571812 }
1858 runFork(forwardShortcut(event, input));
18591813 };
18601814 yield* Scope.addFinalizer(
18611815 scope,
......@@ -1878,6 +1832,9 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
18781832 );
18791833 const install = Effect.fn("PreviewManager.installWebContentsListeners")(function* () {
18801834 yield* attempt({ operation: "attachListeners", tabId, webContentsId: wc.id }, () => {
1835 // Preview input belongs to the page, including keys injected through CDP.
1836 // Never let it invoke the host application's menu accelerators.
1837 wc.setIgnoreMenuShortcuts(true);
18811838 wc.on("did-start-navigation", navigationStarted);
18821839 wc.on("did-navigate", syncNavigation);
18831840 wc.on("did-navigate-in-page", syncInPageNavigation);