diff --git a/packages/app/src/components/browser-webview-resident.browser.test.ts b/packages/app/src/components/browser-webview-resident.browser.test.ts index cca532a449..06b30dfc47 100644 --- a/packages/app/src/components/browser-webview-resident.browser.test.ts +++ b/packages/app/src/components/browser-webview-resident.browser.test.ts @@ -3,11 +3,16 @@ import { type BrowserWebviewProfileHost, clearResidentBrowserWebviewsForTests, ensureResidentBrowserWebview, + getResidentBrowserWebview, prepareBrowserWebview, releaseResidentBrowserWebview, removeResidentBrowserWebview, takeResidentBrowserWebview, } from "./browser-webview-resident"; +import { + setCommandCenterFocusRestoreElement, + takeCommandCenterFocusRestoreElement, +} from "../utils/command-center-focus-restore"; const RESIDENT_HOST_ID = "paseo-browser-resident-webviews"; const attachedBrowsers: Array<{ @@ -254,6 +259,19 @@ describe("resident browser webviews", () => { expect(document.getElementById(RESIDENT_HOST_ID)).toBeNull(); }); + it("finds the originating browser webview for focus restoration", () => { + const webview = ensureResidentBrowserWebview({ + browserId: "browser-focus", + url: "https://example.com", + }); + + setCommandCenterFocusRestoreElement(getResidentBrowserWebview("browser-focus")); + + expect(takeCommandCenterFocusRestoreElement()).toBe(webview); + + expect(getResidentBrowserWebview("browser-missing")).toBeNull(); + }); + it("removes a resident webview when its browser tab closes", () => { const webview = ensureTestBrowser({ browserId: "browser-closed", diff --git a/packages/app/src/components/browser-webview-resident.ts b/packages/app/src/components/browser-webview-resident.ts index 7dfd7c7014..a971900181 100644 --- a/packages/app/src/components/browser-webview-resident.ts +++ b/packages/app/src/components/browser-webview-resident.ts @@ -225,6 +225,19 @@ export function ensureResidentBrowserWebview(input: { return webview; } +export function getResidentBrowserWebview(browserId: string): HTMLElement | null { + const normalizedBrowserId = trimNonEmpty(browserId); + if (!normalizedBrowserId) { + return null; + } + const resident = residentWebviewsByBrowserId.get(normalizedBrowserId) ?? null; + if (resident?.isConnected) { + return resident; + } + const ownerDocument = readDocument(); + return ownerDocument ? findBrowserWebview(normalizedBrowserId, ownerDocument) : null; +} + export function takeResidentBrowserWebview(browserId: string): HTMLElement | null { const normalizedBrowserId = trimNonEmpty(browserId); if (!normalizedBrowserId) { diff --git a/packages/app/src/hooks/use-keyboard-shortcuts.ts b/packages/app/src/hooks/use-keyboard-shortcuts.ts index 75192f7049..56dfb1aa6a 100644 --- a/packages/app/src/hooks/use-keyboard-shortcuts.ts +++ b/packages/app/src/hooks/use-keyboard-shortcuts.ts @@ -1,8 +1,9 @@ -import { useEffect, useMemo, useRef } from "react"; +import { useCallback, useEffect, useMemo, useRef } from "react"; import { usePathname, useRouter } from "expo-router"; import { getIsElectronRuntime } from "@/constants/layout"; import { useKeyboardShortcutsStore } from "@/stores/keyboard-shortcuts-store"; import { setCommandCenterFocusRestoreElement } from "@/utils/command-center-focus-restore"; +import { getResidentBrowserWebview } from "@/components/browser-webview-resident"; import { navigateToWorkspace } from "@/stores/navigation-active-workspace-store"; import { keyboardActionDispatcher } from "@/keyboard/keyboard-action-dispatcher"; import { @@ -64,6 +65,22 @@ export function useKeyboardShortcuts({ const activeWorkspaceSelection = useActiveWorkspaceSelection(); const keyboardWorkspaceSelectionRef = useRef(null); + const publishBrowserShortcutPolicy = useCallback( + (chordState?: ChordState) => { + const prefixes = + enabled && !isMobile + ? buildBrowserShortcutPolicy({ + bindings, + chordState, + isMac, + isDesktop: isDesktopApp, + }) + : []; + void getDesktopHost()?.browser?.setShortcutPolicy?.({ prefixes }); + }, + [bindings, enabled, isDesktopApp, isMac, isMobile], + ); + useEffect(() => { if (activeWorkspaceSelection) { keyboardWorkspaceSelectionRef.current = activeWorkspaceSelection; @@ -75,16 +92,8 @@ export function useKeyboardShortcuts({ return; } - const prefixes = - enabled && !isMobile - ? buildBrowserShortcutPolicy({ - bindings, - isMac, - isDesktop: isDesktopApp, - }) - : []; - void getDesktopHost()?.browser?.setShortcutPolicy?.({ prefixes }); - }, [bindings, enabled, isDesktopApp, isMac, isMobile]); + publishBrowserShortcutPolicy(); + }, [isDesktopApp, publishBrowserShortcutPolicy]); useEffect(() => { if (!enabled) return; @@ -130,6 +139,7 @@ export function useKeyboardShortcuts({ const performShortcutAction = ( action: ShortcutAction, event: KeyboardEvent | null, + browserFocusRestoreElement: HTMLElement | null = null, ): boolean => { switch (action.kind) { case "none": @@ -161,8 +171,12 @@ export function useKeyboardShortcuts({ callbacksByName[action.name]?.(); return true; case "command-center-toggle": { - if (action.nextOpen && event) { - captureCommandCenterFocusRestore(event); + if (action.nextOpen) { + if (event) { + captureCommandCenterFocusRestore(event); + } else { + setCommandCenterFocusRestoreElement(browserFocusRestoreElement); + } } useKeyboardShortcutsStore.getState().setCommandCenterOpen(action.nextOpen); return true; @@ -177,6 +191,7 @@ export function useKeyboardShortcuts({ action: string; payload: KeyboardShortcutPayload; domEvent: KeyboardEvent | null; + browserFocusRestoreElement?: HTMLElement | null; }): boolean => { const store = useKeyboardShortcutsStore.getState(); const shortcutAction = routeKeyboardShortcut( @@ -191,13 +206,18 @@ export function useKeyboardShortcuts({ shortcutsDialogOpen: store.shortcutsDialogOpen, }, ); - return performShortcutAction(shortcutAction, input.domEvent); + return performShortcutAction( + shortcutAction, + input.domEvent, + input.browserFocusRestoreElement, + ); }; const resolveAndPerformShortcut = (input: { event: KeyboardShortcutInput; focusScope: KeyboardFocusScope; domEvent: KeyboardEvent | null; + browserFocusRestoreElement?: HTMLElement | null; }) => { const store = useKeyboardShortcutsStore.getState(); const result = resolveKeyboardShortcut({ @@ -215,11 +235,15 @@ export function useKeyboardShortcuts({ step: 0, timeoutId: null, }; + publishBrowserShortcutPolicy(); }, bindings, }); chordStateRef.current = result.nextChordState; + if ("browserId" in input.event) { + publishBrowserShortcutPolicy(result.nextChordState); + } if (result.preventDefault && input.domEvent) { input.domEvent.preventDefault(); @@ -311,6 +335,7 @@ export function useKeyboardShortcuts({ event: input, focusScope: "browser", domEvent: null, + browserFocusRestoreElement: getResidentBrowserWebview(input.browserId), }); }) : null; @@ -371,6 +396,7 @@ export function useKeyboardShortcuts({ isMobile, openProjectPickerAction, pathname, + publishBrowserShortcutPolicy, resetModifiers, router, toggleAgentList, diff --git a/packages/app/src/keyboard/browser-shortcuts.test.ts b/packages/app/src/keyboard/browser-shortcuts.test.ts index b2b1e945c6..816c086ac2 100644 --- a/packages/app/src/keyboard/browser-shortcuts.test.ts +++ b/packages/app/src/keyboard/browser-shortcuts.test.ts @@ -1,9 +1,9 @@ import { describe, expect, it } from "vitest"; import { buildBrowserShortcutPolicy, parseBrowserShortcutInput } from "./browser-shortcuts"; -import { buildEffectiveBindings } from "./keyboard-shortcuts"; +import { buildEffectiveBindings, resolveKeyboardShortcut } from "./keyboard-shortcuts"; describe("buildBrowserShortcutPolicy", () => { - it("publishes the effective browser page-first shortcut prefixes", () => { + it("publishes only chord starts while no browser chord is pending", () => { const bindings = buildEffectiveBindings({ "workspace-tab-new-ctrl-t-non-mac": "Ctrl+Y", "workspace-terminal-new-ctrl-shift-t-non-mac": "Ctrl+F12 Ctrl+F11", @@ -26,7 +26,7 @@ describe("buildBrowserShortcutPolicy", () => { meta: false, shift: false, }); - expect(policy).toContainEqual({ + expect(policy).not.toContainEqual({ alt: false, code: "F11", control: true, @@ -35,6 +35,55 @@ describe("buildBrowserShortcutPolicy", () => { }); }); + it("publishes a chord continuation only after its browser start crosses the boundary", () => { + const bindings = buildEffectiveBindings({ + "workspace-terminal-new-ctrl-shift-t-non-mac": "Ctrl+F12 Ctrl+F11", + }); + const chordIndex = bindings.findIndex( + (binding) => binding.id === "workspace-terminal-new-ctrl-shift-t-non-mac", + ); + + const policy = buildBrowserShortcutPolicy({ + bindings, + chordState: { candidateIndices: [chordIndex], step: 1, timeoutId: null }, + isMac: false, + isDesktop: true, + }); + + expect(policy).toEqual([ + { + alt: false, + code: "F11", + control: true, + meta: false, + shift: false, + }, + ]); + + const result = resolveKeyboardShortcut({ + event: { + altKey: false, + code: "F11", + ctrlKey: true, + key: "F11", + metaKey: false, + repeat: false, + shiftKey: false, + }, + context: { + commandCenterOpen: false, + focusScope: "browser", + isDesktop: true, + isMac: false, + }, + chordState: { candidateIndices: [chordIndex], step: 1, timeoutId: null }, + onChordReset: () => undefined, + bindings, + }); + + expect(result.match?.action).toBe("workspace.terminal.new"); + }); + it("rejects an entire chord when a continuation cannot cross the browser boundary", () => { const bindings = buildEffectiveBindings({ "settings-toggle-ctrl-comma-non-mac": "Ctrl+F10 F9", diff --git a/packages/app/src/keyboard/browser-shortcuts.ts b/packages/app/src/keyboard/browser-shortcuts.ts index ae27becc2d..75c57b7aff 100644 --- a/packages/app/src/keyboard/browser-shortcuts.ts +++ b/packages/app/src/keyboard/browser-shortcuts.ts @@ -1,4 +1,5 @@ import { + type ChordState, matchesKeyboardShortcutContext, type KeyboardShortcutInput, type ParsedShortcutBinding, @@ -23,6 +24,7 @@ export interface BrowserShortcutInput extends KeyboardShortcutInput { interface BrowserShortcutPolicyInput { bindings: readonly ParsedShortcutBinding[]; + chordState?: ChordState; isMac: boolean; isDesktop: boolean; } @@ -87,19 +89,8 @@ function prefixFromCombo(combo: KeyCombo, isMac: boolean): BrowserShortcutPrefix return prefix.meta || prefix.control || prefix.alt ? prefix : null; } -function prefixesFromBinding( - binding: ParsedShortcutBinding, - isMac: boolean, -): BrowserShortcutPrefix[] | null { - const prefixes: BrowserShortcutPrefix[] = []; - for (const combo of binding.parsedChord) { - const prefix = prefixFromCombo(combo, isMac); - if (!prefix) { - return null; - } - prefixes.push(prefix); - } - return prefixes; +function canCrossBrowserBoundary(binding: ParsedShortcutBinding, isMac: boolean): boolean { + return binding.parsedChord.every((combo) => prefixFromCombo(combo, isMac) !== null); } function prefixKey(prefix: BrowserShortcutPrefix): string { @@ -127,17 +118,32 @@ export function buildBrowserShortcutPolicy( commandCenterOpen: false, }; - for (const binding of input.bindings) { + const candidates = + input.chordState && input.chordState.step > 0 + ? input.chordState.candidateIndices + : input.bindings.map((_, index) => index); + const step = input.chordState?.step ?? 0; + + for (const index of candidates) { + const binding = input.bindings[index]; + if (!binding) { + continue; + } if (!matchesKeyboardShortcutContext(binding.when, context)) { continue; } - const bindingPrefixes = prefixesFromBinding(binding, input.isMac); - if (!bindingPrefixes) { + if (!canCrossBrowserBoundary(binding, input.isMac)) { continue; } - for (const prefix of bindingPrefixes) { - prefixes.set(prefixKey(prefix), prefix); + const combo = binding.parsedChord[step]; + if (!combo) { + continue; } + const prefix = prefixFromCombo(combo, input.isMac); + if (!prefix) { + continue; + } + prefixes.set(prefixKey(prefix), prefix); } return [...prefixes.values()]; diff --git a/packages/desktop/capture-harness/main.js b/packages/desktop/capture-harness/main.js index d10de21ec9..a51928617b 100644 --- a/packages/desktop/capture-harness/main.js +++ b/packages/desktop/capture-harness/main.js @@ -19,6 +19,14 @@ const PRODUCTION_BROWSER_GUEST_PRELOAD_PATH = path.join( "guest-preload.js", ); const PRODUCTION_BROWSER_KEYBOARD_PATH = path.join(PRODUCTION_BROWSER_KEYBOARD_DIR, "index.js"); +const PRODUCTION_BROWSER_WEBVIEW_REGISTRY_PATH = path.join( + ROOT, + "..", + "dist", + "features", + "browser-webviews", + "registry.js", +); const BROWSER_SHORTCUT_INPUT_CHANNEL = "paseo:browser-shortcut-input"; const VIEWPORT_WIDTH = 1280; const VIEWPORT_HEIGHT = 800; @@ -1953,7 +1961,9 @@ async function verifyBrowserKeyboardIsolation({ guest, win, browserId, usesMeta, async function runAutomationGroup() { const results = []; const { BrowserKeyboard } = require(PRODUCTION_BROWSER_KEYBOARD_PATH); - const browserKeyboard = new BrowserKeyboard(); + const { PaseoBrowserWebviewRegistry } = require(PRODUCTION_BROWSER_WEBVIEW_REGISTRY_PATH); + const browserRegistry = new PaseoBrowserWebviewRegistry(); + const browserKeyboard = new BrowserKeyboard(browserRegistry); const browserKeyboardSentinels = installBrowserKeyboardSentinels(); const handle = createInactiveHarnessWindow({ width: 1000, @@ -1994,7 +2004,12 @@ async function runAutomationGroup() { const browserId = "capture-harness-browser"; const usesMeta = process.platform === "darwin"; - browserKeyboard.attach({ browserId, contents: guest, hostContents: win.webContents }); + browserRegistry.registerWebContents({ + browserId, + hostWebContentsId: win.webContents.id, + webContentsId: guest.id, + }); + browserKeyboard.attach({ contents: guest, hostContents: win.webContents }); browserKeyboard.publish(win.webContents.id, { prefixes: [ { diff --git a/packages/desktop/src/features/browser-automation/ipc.ts b/packages/desktop/src/features/browser-automation/ipc.ts index 3f5f8e2967..216e12e96f 100644 --- a/packages/desktop/src/features/browser-automation/ipc.ts +++ b/packages/desktop/src/features/browser-automation/ipc.ts @@ -20,7 +20,7 @@ import { executeAutomationCommand } from "./service.js"; import { listRegisteredPaseoBrowserIds, listRegisteredPaseoBrowserIdsForWorkspace, - getPaseoBrowserWebContents, + getPaseoBrowserWebContentsForHostWindow, getWorkspaceActivePaseoBrowserId, getPaseoBrowserWorkspaceId, } from "../browser-webviews/index.js"; @@ -343,12 +343,12 @@ function normalizeConsoleMessage(input: { }; } -function createRegistry(): BrowserRegistry { +function createRegistry(hostWebContentsId: number): BrowserRegistry { return { listRegisteredBrowserIds: listRegisteredPaseoBrowserIds, listRegisteredBrowserIdsForWorkspace: listRegisteredPaseoBrowserIdsForWorkspace, getTabContents(browserId: string): TabContents | null { - const contents = getPaseoBrowserWebContents(browserId); + const contents = getPaseoBrowserWebContentsForHostWindow(browserId, hostWebContentsId); return contents ? adaptWebContents(contents) : null; }, getBrowserWorkspaceId: getPaseoBrowserWorkspaceId, @@ -358,9 +358,20 @@ function createRegistry(): BrowserRegistry { export function registerBrowserAutomationIpc(options?: { ipc?: IpcHandlerRegistry }): void { const ipc = options?.ipc ?? ipcMain; - const registry = createRegistry(); - ipc.handle("paseo:browser:execute-automation-command", async (_event, rawRequest: unknown) => { + ipc.handle("paseo:browser:execute-automation-command", async (event, rawRequest: unknown) => { + const hostWebContentsId = (event as { sender?: { id?: unknown } }).sender?.id; + if (typeof hostWebContentsId !== "number") { + return { + requestId: readRequestId(rawRequest), + ok: false as const, + error: { + code: "browser_unsupported" as const, + message: "Browser automation requires a host window.", + }, + }; + } + const registry = createRegistry(hostWebContentsId); const parsed = BrowserAutomationExecuteRequestSchema.safeParse(rawRequest); if (!parsed.success) { return { diff --git a/packages/desktop/src/features/browser-keyboard/index.test.ts b/packages/desktop/src/features/browser-keyboard/index.test.ts index b80fb19b31..2a59f7c8b9 100644 --- a/packages/desktop/src/features/browser-keyboard/index.test.ts +++ b/packages/desktop/src/features/browser-keyboard/index.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "vitest"; +import { PaseoBrowserWebviewRegistry } from "../browser-webviews/registry.js"; import { BrowserKeyboard } from "./index.js"; interface SentMessage { @@ -11,7 +12,7 @@ class FakeBrowserContents { public readonly reloads: string[] = []; public readonly sent: SentMessage[] = []; private destroyed = false; - private destroyedListener: (() => void) | null = null; + private readonly destroyedListeners: Array<() => void> = []; private finishLoadListener: (() => void) | null = null; private inputListener: | ((event: { preventDefault(): void }, input: Electron.Input) => void) @@ -36,7 +37,7 @@ class FakeBrowserContents { public once(event: "destroyed", listener: () => void): void { expect(event).toBe("destroyed"); - this.destroyedListener = listener; + this.destroyedListeners.push(listener); } public on(event: "did-finish-load", listener: () => void): void; @@ -80,7 +81,9 @@ class FakeBrowserContents { public destroy(): void { this.destroyed = true; - this.destroyedListener?.(); + for (const listener of this.destroyedListeners) { + listener(); + } } public finishLoad(): void { @@ -101,19 +104,6 @@ class FakeBrowserContents { } } -function shortcutInput(browserId: string) { - return { - alt: false, - browserId, - code: "KeyB", - control: true, - key: "b", - meta: false, - repeat: false, - shift: false, - }; -} - function electronInput(input: Partial): Electron.Input { return { alt: false, @@ -131,96 +121,65 @@ function electronInput(input: Partial): Electron.Input { }; } +function createBrowserKeyboard() { + const registry = new PaseoBrowserWebviewRegistry(); + const keyboard = new BrowserKeyboard(registry); + + function attach(input: { + browserId: string; + contents: FakeBrowserContents; + hostContents: FakeBrowserContents; + }): void { + const webContentsId = input.contents.id; + registry.registerWebContents({ + browserId: input.browserId, + hostWebContentsId: input.hostContents.id, + webContentsId, + }); + input.contents.once("destroyed", () => registry.unregisterWebContents(webContentsId)); + keyboard.attach(input); + } + + return { attach, keyboard }; +} + describe("BrowserKeyboard", () => { - test("forgets a guest after Electron invalidates its wrapper", () => { - const keyboard = new BrowserKeyboard(); - const guest = new FakeBrowserContents(41); - const host = new FakeBrowserContents(42); - const liveContentsWithSameId = new FakeBrowserContents(41); - keyboard.attach({ browserId: "browser-a", contents: guest, hostContents: host }); + test("forwards a validated guest shortcut to its host", () => { + const { attach, keyboard } = createBrowserKeyboard(); + const guest = new FakeBrowserContents(51); + const host = new FakeBrowserContents(52); + attach({ browserId: "browser-a", contents: guest, hostContents: host }); - expect(() => guest.destroy()).not.toThrow(); - keyboard.forwardShortcutInput(liveContentsWithSameId, shortcutInput("browser-a")); - - expect(host.sent).toEqual([]); - }); - - test("does not let a stale destroy event detach a replacement guest", () => { - const keyboard = new BrowserKeyboard(); - const staleGuest = new FakeBrowserContents(51); - const replacementGuest = new FakeBrowserContents(51); - const staleHost = new FakeBrowserContents(52); - const replacementHost = new FakeBrowserContents(53); - keyboard.attach({ + keyboard.forwardShortcutInput(guest, { + alt: false, browserId: "browser-a", - contents: staleGuest, - hostContents: staleHost, + code: "KeyB", + control: true, + key: "b", + meta: false, + repeat: false, + shift: false, }); - keyboard.attach({ - browserId: "browser-a", - contents: replacementGuest, - hostContents: replacementHost, - }); - - staleGuest.destroy(); - keyboard.forwardShortcutInput(replacementGuest, shortcutInput("browser-a")); - - expect(staleHost.sent).toEqual([]); - expect(replacementHost.sent).toEqual([ - { - channel: "paseo:event:browser-shortcut-input", - payload: shortcutInput("browser-a"), - }, - ]); - }); - - test("accepts input only from the authoritative guest for a browser", () => { - const keyboard = new BrowserKeyboard(); - const staleGuest = new FakeBrowserContents(54); - const currentGuest = new FakeBrowserContents(55); - const host = new FakeBrowserContents(56); - keyboard.attach({ browserId: "browser-a", contents: staleGuest, hostContents: host }); - keyboard.attach({ browserId: "browser-a", contents: currentGuest, hostContents: host }); - - keyboard.forwardShortcutInput(staleGuest, shortcutInput("browser-a")); - keyboard.forwardShortcutInput(currentGuest, shortcutInput("browser-a")); expect(host.sent).toEqual([ { channel: "paseo:event:browser-shortcut-input", - payload: shortcutInput("browser-a"), - }, - ]); - }); - - test("keeps same-browser guests active in separate host windows", () => { - const keyboard = new BrowserKeyboard(); - const firstGuest = new FakeBrowserContents(57); - const secondGuest = new FakeBrowserContents(58); - const firstHost = new FakeBrowserContents(59); - const secondHost = new FakeBrowserContents(60); - keyboard.attach({ browserId: "browser-a", contents: firstGuest, hostContents: firstHost }); - keyboard.attach({ browserId: "browser-a", contents: secondGuest, hostContents: secondHost }); - - keyboard.forwardShortcutInput(firstGuest, shortcutInput("browser-a")); - keyboard.forwardShortcutInput(secondGuest, shortcutInput("browser-a")); - - expect(firstHost.sent).toEqual([ - { - channel: "paseo:event:browser-shortcut-input", - payload: shortcutInput("browser-a"), - }, - ]); - expect(secondHost.sent).toEqual([ - { - channel: "paseo:event:browser-shortcut-input", - payload: shortcutInput("browser-a"), + payload: { + alt: false, + browserId: "browser-a", + code: "KeyB", + control: true, + key: "b", + meta: false, + repeat: false, + shift: false, + }, }, ]); }); test("resends the latest shortcut policy after every main-frame load", () => { - const keyboard = new BrowserKeyboard(); + const { attach, keyboard } = createBrowserKeyboard(); const guest = new FakeBrowserContents(61); const host = new FakeBrowserContents(62); const initialPolicy = { @@ -237,7 +196,7 @@ describe("BrowserKeyboard", () => { }; const latestPolicy = { prefixes: [] }; keyboard.publish(host.id, initialPolicy); - keyboard.attach({ browserId: "browser-a", contents: guest, hostContents: host }); + attach({ browserId: "browser-a", contents: guest, hostContents: host }); keyboard.publish(host.id, latestPolicy); guest.finishLoad(); @@ -263,32 +222,11 @@ describe("BrowserKeyboard", () => { ]); }); - test("forgets policy and guests when their host window closes", () => { - const keyboard = new BrowserKeyboard(); - const guest = new FakeBrowserContents(71); - const host = new FakeBrowserContents(72); - const policy = { prefixes: [] }; - keyboard.publish(host.id, policy); - keyboard.attach({ browserId: "browser-a", contents: guest, hostContents: host }); - - keyboard.detachHost(host.id); - guest.finishLoad(); - keyboard.forwardShortcutInput(guest, shortcutInput("browser-a")); - - expect(guest.sent).toEqual([ - { - channel: "paseo:browser-keyboard-policy", - payload: { ...policy, browserId: "browser-a" }, - }, - ]); - expect(host.sent).toEqual([]); - }); - test("owns reserved shortcuts and leaves plain guest input contained", () => { - const keyboard = new BrowserKeyboard(); + const { attach } = createBrowserKeyboard(); const guest = new FakeBrowserContents(81); const host = new FakeBrowserContents(82); - keyboard.attach({ browserId: "browser-a", contents: guest, hostContents: host }); + attach({ browserId: "browser-a", contents: guest, hostContents: host }); const command = process.platform === "darwin" ? { meta: true } : { control: true }; const reservedWasPrevented = guest.input(electronInput({ ...command, code: "KeyT", key: "t" })); @@ -306,7 +244,7 @@ describe("BrowserKeyboard", () => { }); test("keeps policy-owned shortcuts out of the application menu without preempting the page", () => { - const keyboard = new BrowserKeyboard(); + const { attach, keyboard } = createBrowserKeyboard(); const guest = new FakeBrowserContents(91); const host = new FakeBrowserContents(92); keyboard.publish(host.id, { @@ -314,7 +252,7 @@ describe("BrowserKeyboard", () => { { alt: false, code: "KeyW", control: true, meta: false, repeat: false, shift: false }, ], }); - keyboard.attach({ browserId: "browser-a", contents: guest, hostContents: host }); + attach({ browserId: "browser-a", contents: guest, hostContents: host }); const wasPrevented = guest.input(electronInput({ code: "KeyW", control: true, key: "w" })); diff --git a/packages/desktop/src/features/browser-keyboard/index.ts b/packages/desktop/src/features/browser-keyboard/index.ts index 3627a43ef7..689c418db1 100644 --- a/packages/desktop/src/features/browser-keyboard/index.ts +++ b/packages/desktop/src/features/browser-keyboard/index.ts @@ -1,4 +1,5 @@ import { ipcMain } from "electron"; +import { PaseoBrowserWebviewRegistry } from "../browser-webviews/registry.js"; import { type BrowserKeyboardPolicy, classifyBrowserReservedShortcut, @@ -45,18 +46,16 @@ interface BrowserKeyboardHostContents extends BrowserKeyboardContentsIdentity { } interface BrowserKeyboardGuest { - browserId: string; contents: BrowserKeyboardGuestContents; hostContents: BrowserKeyboardHostContents; - hostWebContentsId: number; - webContentsId: number; } export class BrowserKeyboard { - private readonly guestsByHostAndBrowserId = new Map(); - private readonly guestsByWebContentsId = new Map(); + private readonly attachedGuestsByWebContentsId = new Map(); private readonly policiesByHostWebContentsId = new Map(); + public constructor(private readonly browserRegistry: PaseoBrowserWebviewRegistry) {} + public registerIpc(): void { ipcMain.handle(POLICY_INPUT_CHANNEL, (event, rawPolicy: unknown) => { this.publish(event.sender.id, rawPolicy); @@ -67,49 +66,42 @@ export class BrowserKeyboard { } public attach(input: { - browserId: string; contents: BrowserKeyboardGuestContents; hostContents: BrowserKeyboardHostContents; }): void { - const guest: BrowserKeyboardGuest = { - ...input, - hostWebContentsId: input.hostContents.id, - webContentsId: input.contents.id, - }; - const guestAtWebContentsId = this.guestsByWebContentsId.get(guest.webContentsId); - if (guestAtWebContentsId) { - this.detachGuest(guestAtWebContentsId); + const webContentsId = input.contents.id; + const registration = this.browserRegistry.getRegistrationForWebContents(webContentsId); + if (!registration || registration.hostWebContentsId !== input.hostContents.id) { + return; } - const guestForBrowser = this.guestsByHostAndBrowserId.get( - guestKey(guest.hostWebContentsId, guest.browserId), - ); - if (guestForBrowser) { - this.detachGuest(guestForBrowser); - } - this.guestsByHostAndBrowserId.set(guestKey(guest.hostWebContentsId, guest.browserId), guest); - this.guestsByWebContentsId.set(guest.webContentsId, guest); + const guest: BrowserKeyboardGuest = input; + this.attachedGuestsByWebContentsId.set(webContentsId, guest); input.contents.once("destroyed", () => { - this.detachGuest(guest); + if (this.attachedGuestsByWebContentsId.get(webContentsId) === guest) { + this.attachedGuestsByWebContentsId.delete(webContentsId); + } }); input.contents.on("did-finish-load", () => { - if (this.guestsByWebContentsId.get(guest.webContentsId) !== guest) { + const currentRegistration = this.registrationForGuest(webContentsId, guest); + if (!currentRegistration) { return; } - const policy = this.policiesByHostWebContentsId.get(guest.hostWebContentsId); + const policy = this.policiesByHostWebContentsId.get(currentRegistration.hostWebContentsId); if (policy) { - this.sendPolicy(guest, policy); + this.sendPolicy(guest, currentRegistration.browserId, policy); } }); input.contents.on("before-input-event", (event, keyboardInput) => { - if (this.guestsByWebContentsId.get(guest.webContentsId) === guest) { - this.handleGuestInput(guest, event, keyboardInput); + const currentRegistration = this.registrationForGuest(webContentsId, guest); + if (currentRegistration) { + this.handleGuestInput(guest, currentRegistration, event, keyboardInput); } }); - const policy = this.policiesByHostWebContentsId.get(guest.hostWebContentsId); + const policy = this.policiesByHostWebContentsId.get(registration.hostWebContentsId); if (policy) { - this.sendPolicy(guest, policy); + this.sendPolicy(guest, registration.browserId, policy); } } @@ -119,9 +111,10 @@ export class BrowserKeyboard { return; } this.policiesByHostWebContentsId.set(hostWebContentsId, policy); - for (const guest of this.guestsByWebContentsId.values()) { - if (guest.hostWebContentsId === hostWebContentsId) { - this.sendPolicy(guest, policy); + for (const [webContentsId, guest] of this.attachedGuestsByWebContentsId) { + const registration = this.registrationForGuest(webContentsId, guest); + if (registration?.hostWebContentsId === hostWebContentsId) { + this.sendPolicy(guest, registration.browserId, policy); } } } @@ -131,8 +124,16 @@ export class BrowserKeyboard { if (!input) { return; } - const guest = this.guestsByWebContentsId.get(contents.id); - if (!guest || guest.browserId !== input.browserId || guest.hostContents.isDestroyed()) { + const guest = this.attachedGuestsByWebContentsId.get(contents.id); + if (!guest) { + return; + } + const registration = this.registrationForGuest(contents.id, guest); + if ( + !registration || + registration.browserId !== input.browserId || + guest.hostContents.isDestroyed() + ) { return; } guest.hostContents.send(SHORTCUT_OUTPUT_CHANNEL, input); @@ -140,29 +141,15 @@ export class BrowserKeyboard { public detachHost(hostWebContentsId: number): void { this.policiesByHostWebContentsId.delete(hostWebContentsId); - for (const guest of this.guestsByWebContentsId.values()) { - if (guest.hostWebContentsId === hostWebContentsId) { - this.detachGuest(guest); - } - } - } - - private detachGuest(guest: BrowserKeyboardGuest): void { - if (this.guestsByWebContentsId.get(guest.webContentsId) === guest) { - this.guestsByWebContentsId.delete(guest.webContentsId); - } - const key = guestKey(guest.hostWebContentsId, guest.browserId); - if (this.guestsByHostAndBrowserId.get(key) === guest) { - this.guestsByHostAndBrowserId.delete(key); - } } private handleGuestInput( guest: BrowserKeyboardGuest, + registration: { browserId: string; hostWebContentsId: number }, event: BrowserKeyboardInputEvent, input: Electron.Input, ): void { - const policy = this.policiesByHostWebContentsId.get(guest.hostWebContentsId); + const policy = this.policiesByHostWebContentsId.get(registration.hostWebContentsId); const belongsToBrowserPolicy = policy !== undefined && matchesBrowserShortcutPolicy(policy, { @@ -200,7 +187,7 @@ export class BrowserKeyboard { if (!guest.hostContents.isDestroyed()) { guest.hostContents.send(RESERVED_SHORTCUT_OUTPUT_CHANNEL, { action: reservedShortcut, - browserId: guest.browserId, + browserId: registration.browserId, }); } return; @@ -209,13 +196,24 @@ export class BrowserKeyboard { } } - private sendPolicy(guest: BrowserKeyboardGuest, policy: BrowserKeyboardPolicy): void { + private registrationForGuest( + webContentsId: number, + guest: BrowserKeyboardGuest, + ): { browserId: string; hostWebContentsId: number } | null { + if (this.attachedGuestsByWebContentsId.get(webContentsId) !== guest) { + return null; + } + const registration = this.browserRegistry.getRegistrationForWebContents(webContentsId); + return registration?.hostWebContentsId === guest.hostContents.id ? registration : null; + } + + private sendPolicy( + guest: BrowserKeyboardGuest, + browserId: string, + policy: BrowserKeyboardPolicy, + ): void { if (!guest.contents.isDestroyed()) { - guest.contents.send(POLICY_OUTPUT_CHANNEL, { ...policy, browserId: guest.browserId }); + guest.contents.send(POLICY_OUTPUT_CHANNEL, { ...policy, browserId }); } } } - -function guestKey(hostWebContentsId: number, browserId: string): string { - return `${hostWebContentsId}:${browserId}`; -} diff --git a/packages/desktop/src/features/browser-webviews/index.test.ts b/packages/desktop/src/features/browser-webviews/index.test.ts index bbeffbe8c3..7e7cd2c5df 100644 --- a/packages/desktop/src/features/browser-webviews/index.test.ts +++ b/packages/desktop/src/features/browser-webviews/index.test.ts @@ -209,4 +209,28 @@ describe("browser webview attachment", () => { expect(getPaseoBrowserIdForWebContents(guest)).toBeNull(); expect(guest.backgroundThrottlingCalls).toEqual([false]); }); + + test("unregisters a browser only from its requesting host", () => { + const firstContents = new FakeRegisteredWebContents(9003); + const secondContents = new FakeRegisteredWebContents(9004); + registerPaseoBrowserWebContents({ + contents: firstContents, + browserId: "browser-shared-hosts", + hostWebContentsId: 1001, + }); + registerPaseoBrowserWebContents({ + contents: secondContents, + browserId: "browser-shared-hosts", + hostWebContentsId: 1002, + }); + + unregisterPaseoBrowserFromHost(1001, "browser-shared-hosts"); + + expect(getPaseoBrowserIdForWebContents(new LiveWebContentsIdentity(9003))).toBeNull(); + expect(getPaseoBrowserIdForWebContents(new LiveWebContentsIdentity(9004))).toBe( + "browser-shared-hosts", + ); + + unregisterPaseoBrowser("browser-shared-hosts"); + }); });