diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index 79c7fd1725e1..045e0c5ab610 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -67,6 +67,70 @@ describe("isPreviewRefreshShortcut", () => { }); }); +describe("isPreviewEditingShortcut", () => { + const input = (platform: NodeJS.Platform, key: string, overrides: Partial = {}) => + ({ + type: "keyDown", + key, + meta: platform === "darwin", + control: platform !== "darwin", + shift: false, + alt: false, + ...overrides, + }) as Electron.Input; + + it.each(["darwin", "linux", "win32"] as const)( + "allows native editing chords on %s without allowing host shortcuts", + (platform) => { + for (const key of ["a", "c", "v", "x", "z", "V"]) { + expect(PreviewManager.isPreviewEditingShortcut(input(platform, key), platform)).toBe(true); + } + const redo = + platform === "win32" ? input(platform, "y") : input(platform, "z", { shift: true }); + expect(PreviewManager.isPreviewEditingShortcut(redo, platform)).toBe(true); + expect( + PreviewManager.isPreviewEditingShortcut( + input(platform, "v", { shift: true, alt: platform === "darwin" }), + platform, + ), + ).toBe(true); + + for (const key of ["k", ",", "w", "j", "q", "+", "=", "-", "0", "r", "F12"]) { + expect(PreviewManager.isPreviewEditingShortcut(input(platform, key), platform)).toBe(false); + } + for (const modifiers of [ + { meta: false, control: false }, + { meta: true, control: true }, + { meta: platform !== "darwin", control: platform === "darwin" }, + { alt: true }, + { shift: true, alt: platform !== "darwin" }, + ]) { + expect( + PreviewManager.isPreviewEditingShortcut(input(platform, "v", modifiers), platform), + ).toBe(false); + } + expect( + PreviewManager.isPreviewEditingShortcut(input(platform, "a", { shift: true }), platform), + ).toBe(false); + }, + ); + + it("recognizes macOS Paste and Match Style when Option changes the key to a symbol", () => { + const pasteAndMatchStyle = input("darwin", "◊", { code: "KeyV", alt: true, shift: true }); + expect(PreviewManager.isPreviewEditingShortcut(pasteAndMatchStyle, "darwin")).toBe(true); + for (const modifiers of [ + { code: "KeyC" }, + { alt: false }, + { shift: false }, + { control: true }, + ]) { + expect( + PreviewManager.isPreviewEditingShortcut({ ...pasteAndMatchStyle, ...modifiers }, "darwin"), + ).toBe(false); + } + }); +}); + describe("previewWindowOpenAction", () => { const details = (overrides: { readonly url?: string; @@ -484,6 +548,7 @@ describe("PreviewManager", () => { const hostWebContents = { sendInputEvent }; Object.assign(preview.webContents, { hostWebContents }); fromId.mockReturnValue(preview.webContents); + getFocusedWebContents.mockReturnValue(preview.webContents as never); yield* manager.setMainWindow({ isDestroyed: () => false, once: vi.fn(), @@ -497,7 +562,7 @@ describe("PreviewManager", () => { ).toHaveBeenCalledWith(true); const beforeInput = preview.listeners.get("before-input-event")!; for (const control of [false, true]) { - for (const key of ["k", ",", "w", "j", "q", "+", "a", "c", "v", "x"]) { + for (const key of ["k", ",", "w", "j", "q", "+"]) { for (const type of ["keyDown", "keyUp"]) { const preventDefault = vi.fn(); beforeInput( @@ -506,6 +571,9 @@ describe("PreviewManager", () => { ); yield* Effect.yieldNow; expect(preventDefault).not.toHaveBeenCalled(); + expect( + (preview.webContents as Electron.WebContents).setIgnoreMenuShortcuts, + ).toHaveBeenLastCalledWith(true); } } } @@ -527,12 +595,55 @@ describe("PreviewManager", () => { expect(preventDefault).toHaveBeenCalledOnce(); expect(preview.reload).toHaveBeenCalledOnce(); expect(sendInputEvent).not.toHaveBeenCalled(); + }), + ), + ); - const setIgnoreMenuShortcuts = vi.fn(); - preview.listeners.get("did-create-window")!({ - webContents: { setIgnoreMenuShortcuts, setWindowOpenHandler: vi.fn() }, - } as never); - expect(setIgnoreMenuShortcuts).toHaveBeenCalledWith(true); + effectIt.effect("preserves focused browser editing in tabs and sign-in popups", () => + withManager((manager) => + Effect.gen(function* () { + const preview = makeFaviconWebContents(); + fromId.mockReturnValue(preview.webContents); + yield* manager.createTab("tab_editing"); + yield* manager.registerWebview("tab_editing", 42); + + const popup = makeFaviconWebContents({ id: 43 }); + preview.listeners.get("did-create-window")!({ webContents: popup.webContents } as never); + expect( + (popup.webContents as Electron.WebContents).setIgnoreMenuShortcuts, + ).toHaveBeenCalledWith(true); + + for (const browser of [preview, popup]) { + const contents = browser.webContents as Electron.WebContents; + getFocusedWebContents.mockReturnValue(browser.webContents as never); + const beforeInput = browser.listeners.get("before-input-event")!; + const preventDefault = vi.fn(); + const input = { + type: "keyDown", + key: "v", + meta: true, + control: false, + shift: false, + alt: false, + }; + beforeInput({ preventDefault } as never, input as never); + expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(false); + // Releasing Command must not disable native fallback for the pending paste. + beforeInput( + { preventDefault } as never, + { ...input, type: "keyUp", key: "Meta", meta: false } as never, + ); + expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(false); + + beforeInput({ preventDefault } as never, { ...input, key: "w" } as never); + expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(true); + + // An injected paste in an unfocused guest cannot edit the active renderer. + getFocusedWebContents.mockReturnValue(null); + beforeInput({ preventDefault } as never, input as never); + expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(true); + expect(preventDefault).not.toHaveBeenCalled(); + } }), ), ); diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index c82295077fa3..387161bc2721 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -535,6 +535,29 @@ export const isPreviewRefreshShortcut = (input: Electron.Input): boolean => !input.shift && !input.alt; +export const isPreviewEditingShortcut = ( + input: Electron.Input, + platform: NodeJS.Platform, +): boolean => { + const isMac = platform === "darwin"; + if (isMac ? !input.meta || input.control : !input.control || input.meta) return false; + + const key = input.key.toLowerCase(); + // Option changes the DOM key for macOS Paste and Match Style (for example, to ◊). + if (isMac && input.alt && input.shift && input.code === "KeyV") return true; + if (key === "v" && input.shift) return input.alt === isMac; + if (input.alt) return false; + if (key === "z") return !input.shift || platform !== "win32"; + if (input.shift) return false; + return ( + key === "a" || + key === "c" || + key === "v" || + key === "x" || + (key === "y" && platform === "win32") + ); +}; + const isPreviewInputSignal = (value: unknown): value is PreviewInputSignal => { if (typeof value !== "object" || value === null || !("kind" in value)) return false; if (value.kind === "pointer") { @@ -1837,14 +1860,27 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function }).pipe(Effect.ignore), ); }; + const syncMenuShortcuts = (contents: Electron.WebContents, input: Electron.Input): void => { + if (input.type !== "keyDown") return; + // Native editing roles must remain available after the page handles the key. + // Background automation must not edit whichever other renderer has focus. + contents.setIgnoreMenuShortcuts( + !isPreviewEditingShortcut(input, hostPlatform) || + webContents.getFocusedWebContents() !== contents, + ); + }; // A popup opens with Electron's default handler, so the page inside it could // otherwise spawn native windows without limit. Nothing in an OAuth flow // opens a second popup, so the chain stops at the first one. const windowCreated = (window: Electron.BrowserWindow): void => { window.webContents.setIgnoreMenuShortcuts(true); window.webContents.setWindowOpenHandler(() => ({ action: "deny" })); + window.webContents.on("before-input-event", (_event, input) => { + syncMenuShortcuts(window.webContents, input); + }); }; const beforeInput = (event: Electron.Event, input: Electron.Input): void => { + syncMenuShortcuts(wc, input); if (isPreviewRefreshShortcut(input)) { event.preventDefault(); runFork( @@ -1876,8 +1912,8 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function ); const install = Effect.fn("PreviewManager.installWebContentsListeners")(function* () { yield* attempt({ operation: "attachListeners", tabId, webContentsId: wc.id }, () => { - // Preview input belongs to the page, including keys injected through CDP. - // Never let it invoke the host application's menu accelerators. + // Only focused native editing shortcuts may reach the application menu. + // Other preview input, including CDP keys, belongs to the page. wc.setIgnoreMenuShortcuts(true); wc.on("did-start-navigation", navigationStarted); wc.on("did-navigate", syncNavigation);