authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-09-07 17:04:23-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-07 17:04:23-07:00
loge0e0bcb115fa29a2afc670b817fcb4463b620923
treea943c187043cb78b9c973faae6e1b1ee39e9c109
parent8de9169f078fdafedf47d8c299cb8e2c79fda6fb
signature Signed by PGP key B5690EEEBB952194

fix(desktop): preserve browser editing shortcuts (#10621)

2 files changed, 155 insertions(+), 8 deletions(-)

apps/desktop/src/preview/Manager.test.ts+117-6
...@@ -67,6 +67,70 @@ describe("isPreviewRefreshShortcut", () => {...@@ -67,6 +67,70 @@ describe("isPreviewRefreshShortcut", () => {
67 });67 });
68});68});
6969
70describe("isPreviewEditingShortcut", () => {
71 const input = (platform: NodeJS.Platform, key: string, overrides: Partial<Electron.Input> = {}) =>
72 ({
73 type: "keyDown",
74 key,
75 meta: platform === "darwin",
76 control: platform !== "darwin",
77 shift: false,
78 alt: false,
79 ...overrides,
80 }) as Electron.Input;
81
82 it.each(["darwin", "linux", "win32"] as const)(
83 "allows native editing chords on %s without allowing host shortcuts",
84 (platform) => {
85 for (const key of ["a", "c", "v", "x", "z", "V"]) {
86 expect(PreviewManager.isPreviewEditingShortcut(input(platform, key), platform)).toBe(true);
87 }
88 const redo =
89 platform === "win32" ? input(platform, "y") : input(platform, "z", { shift: true });
90 expect(PreviewManager.isPreviewEditingShortcut(redo, platform)).toBe(true);
91 expect(
92 PreviewManager.isPreviewEditingShortcut(
93 input(platform, "v", { shift: true, alt: platform === "darwin" }),
94 platform,
95 ),
96 ).toBe(true);
97
98 for (const key of ["k", ",", "w", "j", "q", "+", "=", "-", "0", "r", "F12"]) {
99 expect(PreviewManager.isPreviewEditingShortcut(input(platform, key), platform)).toBe(false);
100 }
101 for (const modifiers of [
102 { meta: false, control: false },
103 { meta: true, control: true },
104 { meta: platform !== "darwin", control: platform === "darwin" },
105 { alt: true },
106 { shift: true, alt: platform !== "darwin" },
107 ]) {
108 expect(
109 PreviewManager.isPreviewEditingShortcut(input(platform, "v", modifiers), platform),
110 ).toBe(false);
111 }
112 expect(
113 PreviewManager.isPreviewEditingShortcut(input(platform, "a", { shift: true }), platform),
114 ).toBe(false);
115 },
116 );
117
118 it("recognizes macOS Paste and Match Style when Option changes the key to a symbol", () => {
119 const pasteAndMatchStyle = input("darwin", "◊", { code: "KeyV", alt: true, shift: true });
120 expect(PreviewManager.isPreviewEditingShortcut(pasteAndMatchStyle, "darwin")).toBe(true);
121 for (const modifiers of [
122 { code: "KeyC" },
123 { alt: false },
124 { shift: false },
125 { control: true },
126 ]) {
127 expect(
128 PreviewManager.isPreviewEditingShortcut({ ...pasteAndMatchStyle, ...modifiers }, "darwin"),
129 ).toBe(false);
130 }
131 });
132});
133
70describe("previewWindowOpenAction", () => {134describe("previewWindowOpenAction", () => {
71 const details = (overrides: {135 const details = (overrides: {
72 readonly url?: string;136 readonly url?: string;
...@@ -484,6 +548,7 @@ describe("PreviewManager", () => {...@@ -484,6 +548,7 @@ describe("PreviewManager", () => {
484 const hostWebContents = { sendInputEvent };548 const hostWebContents = { sendInputEvent };
485 Object.assign(preview.webContents, { hostWebContents });549 Object.assign(preview.webContents, { hostWebContents });
486 fromId.mockReturnValue(preview.webContents);550 fromId.mockReturnValue(preview.webContents);
551 getFocusedWebContents.mockReturnValue(preview.webContents as never);
487 yield* manager.setMainWindow({552 yield* manager.setMainWindow({
488 isDestroyed: () => false,553 isDestroyed: () => false,
489 once: vi.fn(),554 once: vi.fn(),
...@@ -497,7 +562,7 @@ describe("PreviewManager", () => {...@@ -497,7 +562,7 @@ describe("PreviewManager", () => {
497 ).toHaveBeenCalledWith(true);562 ).toHaveBeenCalledWith(true);
498 const beforeInput = preview.listeners.get("before-input-event")!;563 const beforeInput = preview.listeners.get("before-input-event")!;
499 for (const control of [false, true]) {564 for (const control of [false, true]) {
500 for (const key of ["k", ",", "w", "j", "q", "+", "a", "c", "v", "x"]) {565 for (const key of ["k", ",", "w", "j", "q", "+"]) {
501 for (const type of ["keyDown", "keyUp"]) {566 for (const type of ["keyDown", "keyUp"]) {
502 const preventDefault = vi.fn();567 const preventDefault = vi.fn();
503 beforeInput(568 beforeInput(
...@@ -506,6 +571,9 @@ describe("PreviewManager", () => {...@@ -506,6 +571,9 @@ describe("PreviewManager", () => {
506 );571 );
507 yield* Effect.yieldNow;572 yield* Effect.yieldNow;
508 expect(preventDefault).not.toHaveBeenCalled();573 expect(preventDefault).not.toHaveBeenCalled();
574 expect(
575 (preview.webContents as Electron.WebContents).setIgnoreMenuShortcuts,
576 ).toHaveBeenLastCalledWith(true);
509 }577 }
510 }578 }
511 }579 }
...@@ -527,12 +595,55 @@ describe("PreviewManager", () => {...@@ -527,12 +595,55 @@ describe("PreviewManager", () => {
527 expect(preventDefault).toHaveBeenCalledOnce();595 expect(preventDefault).toHaveBeenCalledOnce();
528 expect(preview.reload).toHaveBeenCalledOnce();596 expect(preview.reload).toHaveBeenCalledOnce();
529 expect(sendInputEvent).not.toHaveBeenCalled();597 expect(sendInputEvent).not.toHaveBeenCalled();
598 }),
599 ),
600 );
530601
531 const setIgnoreMenuShortcuts = vi.fn();602 effectIt.effect("preserves focused browser editing in tabs and sign-in popups", () =>
532 preview.listeners.get("did-create-window")!({603 withManager((manager) =>
533 webContents: { setIgnoreMenuShortcuts, setWindowOpenHandler: vi.fn() },604 Effect.gen(function* () {
534 } as never);605 const preview = makeFaviconWebContents();
535 expect(setIgnoreMenuShortcuts).toHaveBeenCalledWith(true);606 fromId.mockReturnValue(preview.webContents);
607 yield* manager.createTab("tab_editing");
608 yield* manager.registerWebview("tab_editing", 42);
609
610 const popup = makeFaviconWebContents({ id: 43 });
611 preview.listeners.get("did-create-window")!({ webContents: popup.webContents } as never);
612 expect(
613 (popup.webContents as Electron.WebContents).setIgnoreMenuShortcuts,
614 ).toHaveBeenCalledWith(true);
615
616 for (const browser of [preview, popup]) {
617 const contents = browser.webContents as Electron.WebContents;
618 getFocusedWebContents.mockReturnValue(browser.webContents as never);
619 const beforeInput = browser.listeners.get("before-input-event")!;
620 const preventDefault = vi.fn();
621 const input = {
622 type: "keyDown",
623 key: "v",
624 meta: true,
625 control: false,
626 shift: false,
627 alt: false,
628 };
629 beforeInput({ preventDefault } as never, input as never);
630 expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(false);
631 // Releasing Command must not disable native fallback for the pending paste.
632 beforeInput(
633 { preventDefault } as never,
634 { ...input, type: "keyUp", key: "Meta", meta: false } as never,
635 );
636 expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(false);
637
638 beforeInput({ preventDefault } as never, { ...input, key: "w" } as never);
639 expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(true);
640
641 // An injected paste in an unfocused guest cannot edit the active renderer.
642 getFocusedWebContents.mockReturnValue(null);
643 beforeInput({ preventDefault } as never, input as never);
644 expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(true);
645 expect(preventDefault).not.toHaveBeenCalled();
646 }
536 }),647 }),
537 ),648 ),
538 );649 );
apps/desktop/src/preview/Manager.ts+38-2
...@@ -535,6 +535,29 @@ export const isPreviewRefreshShortcut = (input: Electron.Input): boolean =>...@@ -535,6 +535,29 @@ export const isPreviewRefreshShortcut = (input: Electron.Input): boolean =>
535 !input.shift &&535 !input.shift &&
536 !input.alt;536 !input.alt;
537537
538export const isPreviewEditingShortcut = (
539 input: Electron.Input,
540 platform: NodeJS.Platform,
541): boolean => {
542 const isMac = platform === "darwin";
543 if (isMac ? !input.meta || input.control : !input.control || input.meta) return false;
544
545 const key = input.key.toLowerCase();
546 // Option changes the DOM key for macOS Paste and Match Style (for example, to ◊).
547 if (isMac && input.alt && input.shift && input.code === "KeyV") return true;
548 if (key === "v" && input.shift) return input.alt === isMac;
549 if (input.alt) return false;
550 if (key === "z") return !input.shift || platform !== "win32";
551 if (input.shift) return false;
552 return (
553 key === "a" ||
554 key === "c" ||
555 key === "v" ||
556 key === "x" ||
557 (key === "y" && platform === "win32")
558 );
559};
560
538const isPreviewInputSignal = (value: unknown): value is PreviewInputSignal => {561const isPreviewInputSignal = (value: unknown): value is PreviewInputSignal => {
539 if (typeof value !== "object" || value === null || !("kind" in value)) return false;562 if (typeof value !== "object" || value === null || !("kind" in value)) return false;
540 if (value.kind === "pointer") {563 if (value.kind === "pointer") {
...@@ -1837,14 +1860,27 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function...@@ -1837,14 +1860,27 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
1837 }).pipe(Effect.ignore),1860 }).pipe(Effect.ignore),
1838 );1861 );
1839 };1862 };
1863 const syncMenuShortcuts = (contents: Electron.WebContents, input: Electron.Input): void => {
1864 if (input.type !== "keyDown") return;
1865 // Native editing roles must remain available after the page handles the key.
1866 // Background automation must not edit whichever other renderer has focus.
1867 contents.setIgnoreMenuShortcuts(
1868 !isPreviewEditingShortcut(input, hostPlatform) ||
1869 webContents.getFocusedWebContents() !== contents,
1870 );
1871 };
1840 // A popup opens with Electron's default handler, so the page inside it could1872 // A popup opens with Electron's default handler, so the page inside it could
1841 // otherwise spawn native windows without limit. Nothing in an OAuth flow1873 // otherwise spawn native windows without limit. Nothing in an OAuth flow
1842 // opens a second popup, so the chain stops at the first one.1874 // opens a second popup, so the chain stops at the first one.
1843 const windowCreated = (window: Electron.BrowserWindow): void => {1875 const windowCreated = (window: Electron.BrowserWindow): void => {
1844 window.webContents.setIgnoreMenuShortcuts(true);1876 window.webContents.setIgnoreMenuShortcuts(true);
1845 window.webContents.setWindowOpenHandler(() => ({ action: "deny" }));1877 window.webContents.setWindowOpenHandler(() => ({ action: "deny" }));
1878 window.webContents.on("before-input-event", (_event, input) => {
1879 syncMenuShortcuts(window.webContents, input);
1880 });
1846 };1881 };
1847 const beforeInput = (event: Electron.Event, input: Electron.Input): void => {1882 const beforeInput = (event: Electron.Event, input: Electron.Input): void => {
1883 syncMenuShortcuts(wc, input);
1848 if (isPreviewRefreshShortcut(input)) {1884 if (isPreviewRefreshShortcut(input)) {
1849 event.preventDefault();1885 event.preventDefault();
1850 runFork(1886 runFork(
...@@ -1876,8 +1912,8 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function...@@ -1876,8 +1912,8 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
1876 );1912 );
1877 const install = Effect.fn("PreviewManager.installWebContentsListeners")(function* () {1913 const install = Effect.fn("PreviewManager.installWebContentsListeners")(function* () {
1878 yield* attempt({ operation: "attachListeners", tabId, webContentsId: wc.id }, () => {1914 yield* attempt({ operation: "attachListeners", tabId, webContentsId: wc.id }, () => {
1879 // Preview input belongs to the page, including keys injected through CDP.1915 // Only focused native editing shortcuts may reach the application menu.
1880 // Never let it invoke the host application's menu accelerators.1916 // Other preview input, including CDP keys, belongs to the page.
1881 wc.setIgnoreMenuShortcuts(true);1917 wc.setIgnoreMenuShortcuts(true);
1882 wc.on("did-start-navigation", navigationStarted);1918 wc.on("did-start-navigation", navigationStarted);
1883 wc.on("did-navigate", syncNavigation);1919 wc.on("did-navigate", syncNavigation);