diff --git a/src/main/browser/browser-clicked-link-routing.test.ts b/src/main/browser/browser-clicked-link-routing.test.ts deleted file mode 100644 index 315d6ae1779..00000000000 --- a/src/main/browser/browser-clicked-link-routing.test.ts +++ /dev/null @@ -1,344 +0,0 @@ -// @vitest-environment happy-dom - -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' - -import { - buildBrowserClickedLinkRoutingScript, - buildBrowserIframeClickedLinkRoutingScript, - installBrowserIframeClickedLinkRouting, - installBrowserClickedLinkRouting -} from './browser-clicked-link-routing' - -const FOREGROUND_FRAME_NAME = '__orca_clicked_link_foreground_test' -const BACKGROUND_FRAME_NAME = '__orca_clicked_link_background_test' - -type RoutingGlobal = typeof globalThis & { - __orcaBrowserClickedLinkRouting?: { - listener: (event: MouseEvent) => void - } -} - -let cleanupIframeRouting: (() => void) | null = null - -function resetRouting(): void { - const routingGlobal = globalThis as RoutingGlobal - const state = routingGlobal.__orcaBrowserClickedLinkRouting - if (state) { - window.removeEventListener('click', state.listener) - window.removeEventListener('auxclick', state.listener) - } - delete routingGlobal.__orcaBrowserClickedLinkRouting - cleanupIframeRouting?.() - cleanupIframeRouting = null -} - -function installRouting(isMac = true): void { - installBrowserClickedLinkRouting(FOREGROUND_FRAME_NAME, BACKGROUND_FRAME_NAME, isMac, true) -} - -function clickLink( - link: Element, - init: MouseEventInit & { type?: 'click' | 'auxclick' } = {} -): { event: MouseEvent; open: ReturnType } { - const open = vi.fn() - vi.spyOn(window, 'open').mockImplementation(open) - const { type = 'click', ...eventInit } = init - const event = new MouseEvent(type, { - bubbles: true, - cancelable: true, - button: type === 'auxclick' ? 1 : 0, - ...eventInit - }) - link.dispatchEvent(event) - return { event, open } -} - -describe('browser clicked-link routing', () => { - beforeEach(() => { - resetRouting() - document.head.innerHTML = '' - document.body.innerHTML = '' - }) - - afterEach(() => { - resetRouting() - vi.restoreAllMocks() - }) - - it('routes plain target=_blank links into a new Orca tab', () => { - const link = document.createElement('a') - link.href = 'https://docs.example.com/guide' - link.target = '_blank' - document.body.append(link) - installRouting() - - const { event, open } = clickLink(link) - - expect(event.defaultPrevented).toBe(true) - expect(open).toHaveBeenCalledWith('https://docs.example.com/guide', FOREGROUND_FRAME_NAME) - }) - - it('routes the host-platform modifier without trusting an emulated guest user agent', () => { - Object.defineProperty(navigator, 'userAgent', { - configurable: true, - value: 'Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X)' - }) - const link = document.createElement('a') - link.href = 'https://example.com/reference' - document.body.append(link) - installRouting(false) - - expect(clickLink(link, { metaKey: true }).open).not.toHaveBeenCalled() - expect(clickLink(link, { ctrlKey: true }).open).toHaveBeenCalledWith( - 'https://example.com/reference', - BACKGROUND_FRAME_NAME - ) - }) - - it('routes middle clicks and primary-modifier plus Shift clicks as new-tab intents', () => { - const link = document.createElement('a') - link.href = 'https://example.com/reference' - document.body.append(link) - installRouting() - - expect(clickLink(link, { type: 'auxclick' }).open).toHaveBeenCalledWith( - 'https://example.com/reference', - BACKGROUND_FRAME_NAME - ) - expect(clickLink(link, { metaKey: true, shiftKey: true }).open).toHaveBeenCalledWith( - 'https://example.com/reference', - FOREGROUND_FRAME_NAME - ) - }) - - it.each([true, false])('routes Shift+middle-click to the foreground (isMac=%s)', (isMac) => { - const link = document.createElement('a') - link.href = 'https://example.com/reference' - document.body.append(link) - installRouting(isMac) - - const { event, open } = clickLink(link, { type: 'auxclick', shiftKey: true }) - expect(event.defaultPrevented).toBe(true) - expect(open).toHaveBeenCalledWith(link.href, FOREGROUND_FRAME_NAME) - - resetRouting() - cleanupIframeRouting = installBrowserIframeClickedLinkRouting( - FOREGROUND_FRAME_NAME, - BACKGROUND_FRAME_NAME, - isMac, - true - ) - const iframeClick = clickLink(link, { type: 'auxclick', shiftKey: true }) - expect(iframeClick.event.defaultPrevented).toBe(true) - expect(iframeClick.open).toHaveBeenCalledWith(link.href, FOREGROUND_FRAME_NAME) - }) - - it('honors page cancellation and observes link rewrites before routing', () => { - const cancelled = document.createElement('a') - cancelled.href = 'https://example.com/original' - cancelled.target = '_blank' - cancelled.addEventListener('click', (event) => event.preventDefault()) - const rewritten = document.createElement('a') - rewritten.href = 'https://example.com/original' - rewritten.target = '_blank' - rewritten.addEventListener('click', () => { - rewritten.href = 'https://example.com/rewritten' - }) - document.body.append(cancelled, rewritten) - installRouting() - - expect(clickLink(cancelled).open).not.toHaveBeenCalled() - expect(cancelled.getAttribute('target')).toBe('_blank') - // The page's own handler ran first, so routing must follow the rewritten href. - expect(clickLink(rewritten).open).toHaveBeenCalledWith( - 'https://example.com/rewritten', - FOREGROUND_FRAME_NAME - ) - }) - - it('routes SVG links but leaves download links and links without href alone', () => { - const svgLink = document.createElementNS('http://www.w3.org/2000/svg', 'a') - svgLink.setAttribute('href', 'https://example.com/svg') - svgLink.setAttribute('target', '_blank') - const areaDownload = document.createElement('area') - areaDownload.href = 'https://example.com/archive.zip' - areaDownload.target = '_blank' - areaDownload.download = 'archive.zip' - const noHref = document.createElement('a') - noHref.target = '_blank' - document.body.append(svgLink, areaDownload, noHref) - installRouting() - - expect(clickLink(svgLink).open).toHaveBeenCalledWith( - 'https://example.com/svg', - FOREGROUND_FRAME_NAME - ) - expect(clickLink(areaDownload).open).not.toHaveBeenCalled() - expect(clickLink(noHref).open).not.toHaveBeenCalled() - }) - - it('leaves ordinary same-tab, Shift-window, Alt, and unsafe links to Chromium', () => { - const sameTab = document.createElement('a') - sameTab.href = 'https://example.com/current' - const shifted = document.createElement('a') - shifted.href = 'https://example.com/window' - shifted.target = '_blank' - const altClicked = document.createElement('a') - altClicked.href = 'https://example.com/download' - altClicked.target = '_blank' - const unsafe = document.createElement('a') - unsafe.href = 'javascript:void(0)' - unsafe.target = '_blank' - document.body.append(sameTab, shifted, altClicked, unsafe) - installRouting() - - expect(clickLink(sameTab).open).not.toHaveBeenCalled() - expect(clickLink(shifted, { shiftKey: true }).open).not.toHaveBeenCalled() - expect(clickLink(altClicked, { altKey: true }).open).not.toHaveBeenCalled() - expect(clickLink(unsafe).open).not.toHaveBeenCalled() - }) - - it('updates an existing isolated-world installation without adding another listener', () => { - const addEventListener = vi.spyOn(window, 'addEventListener') - const link = document.createElement('a') - link.href = 'https://example.com/reference' - document.body.append(link) - installBrowserClickedLinkRouting( - '__orca_clicked_link_old_fg', - '__orca_clicked_link_old_bg', - true, - true - ) - installBrowserClickedLinkRouting( - '__orca_clicked_link_new_fg', - '__orca_clicked_link_new_bg', - false, - true - ) - - expect(addEventListener.mock.calls.filter(([event]) => event === 'click')).toHaveLength(1) - expect(clickLink(link, { ctrlKey: true }).open).toHaveBeenCalledWith( - 'https://example.com/reference', - '__orca_clicked_link_new_bg' - ) - }) - - it('builds a self-contained isolated-world script', () => { - const script = buildBrowserClickedLinkRoutingScript( - FOREGROUND_FRAME_NAME, - BACKGROUND_FRAME_NAME, - false - ) - - expect(script).toContain('installBrowserClickedLinkRouting') - expect(script).toContain(`"${FOREGROUND_FRAME_NAME}","${BACKGROUND_FRAME_NAME}",false`) - expect(script).not.toContain('BrowserClickedLinkRoutingState') - }) - - it('routes plain iframe target=_blank links into a new Orca tab', () => { - const link = document.createElement('a') - link.href = 'https://example.com/from-frame' - link.target = '_blank' - document.body.append(link) - cleanupIframeRouting = installBrowserIframeClickedLinkRouting( - FOREGROUND_FRAME_NAME, - BACKGROUND_FRAME_NAME, - true, - true - ) - - const { event, open } = clickLink(link) - - expect(event.defaultPrevented).toBe(true) - expect(open).toHaveBeenCalledWith('https://example.com/from-frame', FOREGROUND_FRAME_NAME) - }) - - it('routes explicit iframe new-tab gestures through one-use frame names', () => { - const blank = document.createElement('a') - blank.href = 'https://example.com/new-context' - blank.target = '_blank' - document.body.append(blank) - cleanupIframeRouting = installBrowserIframeClickedLinkRouting( - FOREGROUND_FRAME_NAME, - BACKGROUND_FRAME_NAME, - true, - true - ) - - expect(clickLink(blank, { metaKey: true }).open).toHaveBeenCalledWith( - 'https://example.com/new-context', - BACKGROUND_FRAME_NAME - ) - - cleanupIframeRouting = installBrowserIframeClickedLinkRouting( - FOREGROUND_FRAME_NAME, - BACKGROUND_FRAME_NAME, - true, - true - ) - expect(clickLink(blank, { type: 'auxclick' }).open).toHaveBeenCalledWith( - 'https://example.com/new-context', - BACKGROUND_FRAME_NAME - ) - - cleanupIframeRouting = installBrowserIframeClickedLinkRouting( - FOREGROUND_FRAME_NAME, - BACKGROUND_FRAME_NAME, - true, - true - ) - expect(clickLink(blank, { metaKey: true, shiftKey: true }).open).toHaveBeenCalledWith( - 'https://example.com/new-context', - FOREGROUND_FRAME_NAME - ) - }) - - it('leaves other-platform, Shift-window, and ordinary iframe clicks to Chromium', () => { - const blank = document.createElement('a') - blank.href = 'https://example.com/new-context' - blank.target = '_blank' - const ordinary = document.createElement('a') - ordinary.href = 'https://example.com/current' - document.body.append(blank, ordinary) - cleanupIframeRouting = installBrowserIframeClickedLinkRouting( - FOREGROUND_FRAME_NAME, - BACKGROUND_FRAME_NAME, - true, - true - ) - - expect(clickLink(blank, { ctrlKey: true }).open).not.toHaveBeenCalled() - expect(clickLink(blank, { shiftKey: true }).open).not.toHaveBeenCalled() - expect(clickLink(ordinary).open).not.toHaveBeenCalled() - }) - - it('ignores synthetic main-frame and iframe clicks in production mode', () => { - const mainLink = document.createElement('a') - mainLink.href = 'https://example.com/main-synthetic' - const frameLink = document.createElement('a') - frameLink.href = 'https://example.com/frame-synthetic' - frameLink.target = '_blank' - document.body.append(mainLink, frameLink) - installBrowserClickedLinkRouting(FOREGROUND_FRAME_NAME, BACKGROUND_FRAME_NAME, true) - cleanupIframeRouting = installBrowserIframeClickedLinkRouting( - FOREGROUND_FRAME_NAME, - BACKGROUND_FRAME_NAME, - true - ) - - expect(clickLink(mainLink, { metaKey: true }).open).not.toHaveBeenCalled() - expect(clickLink(frameLink).open).not.toHaveBeenCalled() - expect(frameLink.getAttribute('target')).toBe('_blank') - }) - - it('builds a self-contained iframe script with per-frame routing tokens', () => { - const script = buildBrowserIframeClickedLinkRoutingScript( - FOREGROUND_FRAME_NAME, - BACKGROUND_FRAME_NAME, - false - ) - - expect(script).toContain('installBrowserIframeClickedLinkRouting') - expect(script).toContain(`"${FOREGROUND_FRAME_NAME}","${BACKGROUND_FRAME_NAME}",false`) - }) -}) diff --git a/src/main/browser/browser-clicked-link-routing.ts b/src/main/browser/browser-clicked-link-routing.ts deleted file mode 100644 index 4189f222ef8..00000000000 --- a/src/main/browser/browser-clicked-link-routing.ts +++ /dev/null @@ -1,223 +0,0 @@ -export const BROWSER_CLICKED_LINK_ROUTING_WORLD_ID = 1208 - -export type BrowserClickedLinkFrameNames = { - foreground: string - background: string -} - -type BrowserClickedLinkRoutingState = { - foregroundFrameName: string - backgroundFrameName: string - isMac: boolean - allowUntrustedEvents: boolean - listener: (event: MouseEvent) => void -} - -type BrowserClickedLinkRoutingGlobal = typeof globalThis & { - __orcaBrowserClickedLinkRouting?: BrowserClickedLinkRoutingState -} - -/** - * Re-expresses explicit new-tab link gestures with private frame names so main - * can distinguish them from opener-dependent window.open calls. - */ -export function installBrowserClickedLinkRouting( - foregroundFrameName: string, - backgroundFrameName: string, - isMac: boolean, - allowUntrustedEvents = false -): void { - const routingGlobal = globalThis as BrowserClickedLinkRoutingGlobal - const existing = routingGlobal.__orcaBrowserClickedLinkRouting - if (existing) { - existing.foregroundFrameName = foregroundFrameName - existing.backgroundFrameName = backgroundFrameName - existing.isMac = isMac - existing.allowUntrustedEvents = allowUntrustedEvents - return - } - - const state: BrowserClickedLinkRoutingState = { - foregroundFrameName, - backgroundFrameName, - isMac, - allowUntrustedEvents, - listener: () => {} - } - state.listener = (event) => { - const primaryClick = event.type === 'click' && event.button === 0 - const middleClick = event.type === 'auxclick' && event.button === 1 - if ( - !(event instanceof MouseEvent) || - (!event.isTrusted && !state.allowUntrustedEvents) || - (!primaryClick && !middleClick) || - event.defaultPrevented || - event.altKey - ) { - return - } - - const link = event - .composedPath() - .find( - (target): target is Element => - target instanceof Element && - ((target.namespaceURI === 'http://www.w3.org/1999/xhtml' && - (target.localName === 'a' || target.localName === 'area')) || - (target.namespaceURI === 'http://www.w3.org/2000/svg' && target.localName === 'a')) - ) - if (!link || link.hasAttribute('download')) { - return - } - - const modifierClick = state.isMac ? event.metaKey : event.ctrlKey - const otherPlatformModifier = state.isMac ? event.ctrlKey : event.metaKey - if (otherPlatformModifier) { - return - } - // Shift alone is browser-native new-window intent; keep OAuth and other - // opener-dependent window flows in Orca's guarded popup window. - if (event.shiftKey && !modifierClick && !middleClick) { - return - } - - const baseTarget = document.querySelector('base[target]')?.getAttribute('target') ?? '' - const ownTarget = link.getAttribute('target') - const effectiveTarget = (ownTarget === null ? baseTarget : ownTarget).trim().toLowerCase() - // target=_blank is a new-tab request, exactly as it is in every other - // browser; the modifiers are the other two ways to ask for one. - if (!(middleClick || modifierClick || effectiveTarget === '_blank')) { - return - } - - const rawHref = - link.getAttribute('href') ?? link.getAttributeNS('http://www.w3.org/1999/xlink', 'href') - if (rawHref === null) { - return - } - - let targetUrl: URL - try { - targetUrl = new URL(rawHref, document.baseURI) - } catch { - return - } - if (targetUrl.protocol !== 'http:' && targetUrl.protocol !== 'https:') { - return - } - - // Why: Electron reports direct link clicks and featureless window.open() - // with the same disposition. The private frame name preserves that one - // distinction without weakening OAuth popups that need window.opener. - event.preventDefault() - const openInBackground = (middleClick || modifierClick) && !event.shiftKey - window.open( - targetUrl.toString(), - openInBackground ? state.backgroundFrameName : state.foregroundFrameName - ) - } - routingGlobal.__orcaBrowserClickedLinkRouting = state - - // Why: page click handlers must get the first chance to cancel or rewrite a - // link; capture-phase interception breaks SPA routing and analytics handlers. - window.addEventListener('click', state.listener, false) - window.addEventListener('auxclick', state.listener, false) -} - -/** - * Same routing for links inside child frames, which Electron's isolated-world - * API cannot reach, so this runs in the page world against a one-use token. - */ -export function installBrowserIframeClickedLinkRouting( - foregroundFrameName: string, - backgroundFrameName: string, - isMac: boolean, - allowUntrustedEvents = false -): () => void { - const listener = (event: MouseEvent): void => { - const primaryClick = event.type === 'click' && event.button === 0 - const middleClick = event.type === 'auxclick' && event.button === 1 - if ( - !(event instanceof MouseEvent) || - (!event.isTrusted && !allowUntrustedEvents) || - (!primaryClick && !middleClick) || - event.defaultPrevented || - event.altKey - ) { - return - } - - const link = event - .composedPath() - .find( - (target): target is Element => - target instanceof Element && - ((target.namespaceURI === 'http://www.w3.org/1999/xhtml' && - (target.localName === 'a' || target.localName === 'area')) || - (target.namespaceURI === 'http://www.w3.org/2000/svg' && target.localName === 'a')) - ) - if (!link || link.hasAttribute('download')) { - return - } - - const modifierClick = isMac ? event.metaKey : event.ctrlKey - const otherPlatformModifier = isMac ? event.ctrlKey : event.metaKey - if (otherPlatformModifier || (event.shiftKey && !modifierClick && !middleClick)) { - return - } - - const baseTarget = document.querySelector('base[target]')?.getAttribute('target') ?? '' - const ownTarget = link.getAttribute('target') - const effectiveTarget = (ownTarget === null ? baseTarget : ownTarget).trim().toLowerCase() - if (!(middleClick || modifierClick || effectiveTarget === '_blank')) { - return - } - - const rawHref = - link.getAttribute('href') ?? link.getAttributeNS('http://www.w3.org/1999/xlink', 'href') - if (rawHref === null) { - return - } - - let targetUrl: URL - try { - targetUrl = new URL(rawHref, document.baseURI) - } catch { - return - } - if (targetUrl.protocol !== 'http:' && targetUrl.protocol !== 'https:') { - return - } - - // Why: child-frame code runs in the page world, so each token is one-use. - // A page that observes a real click cannot replay it to create more tabs. - event.preventDefault() - cleanup() - const openInBackground = (middleClick || modifierClick) && !event.shiftKey - window.open(targetUrl.toString(), openInBackground ? backgroundFrameName : foregroundFrameName) - } - - const cleanup = (): void => { - window.removeEventListener('click', listener, false) - window.removeEventListener('auxclick', listener, false) - } - window.addEventListener('click', listener, false) - window.addEventListener('auxclick', listener, false) - return cleanup -} - -export function buildBrowserClickedLinkRoutingScript( - foregroundFrameName: string, - backgroundFrameName: string, - isMac: boolean -): string { - return `(${installBrowserClickedLinkRouting.toString()})(${JSON.stringify(foregroundFrameName)},${JSON.stringify(backgroundFrameName)},${JSON.stringify(isMac)});` -} - -export function buildBrowserIframeClickedLinkRoutingScript( - foregroundFrameName: string, - backgroundFrameName: string, - isMac: boolean -): string { - return `void (${installBrowserIframeClickedLinkRouting.toString()})(${JSON.stringify(foregroundFrameName)},${JSON.stringify(backgroundFrameName)},${JSON.stringify(isMac)});` -} diff --git a/src/main/browser/browser-route-guest-popup-gesture.ts b/src/main/browser/browser-guest-input-gesture.ts similarity index 72% rename from src/main/browser/browser-route-guest-popup-gesture.ts rename to src/main/browser/browser-guest-input-gesture.ts index 85396b9d788..49b2ded2ea8 100644 --- a/src/main/browser/browser-route-guest-popup-gesture.ts +++ b/src/main/browser/browser-guest-input-gesture.ts @@ -15,27 +15,27 @@ const ACTIVATING_INPUT_TYPES: ReadonlySet = new Set([ 'pointerUp' ]) -export const BROWSER_ROUTE_GUEST_POPUP_GESTURE_WINDOW_MS = 1_000 +export const BROWSER_GUEST_INPUT_GESTURE_WINDOW_MS = 1_000 -export type BrowserRouteGuestPopupGesture = { - /** Single-use: one observed click cannot be replayed into a second popup. */ +export type BrowserGuestInputGesture = { + /** Single-use: one observed click cannot be replayed into a second window or tab. */ consume: () => boolean dispose: () => void } -export function trackBrowserRouteGuestPopupGesture( +export function trackBrowserGuestInputGesture( guest: WebContents, now: () => number = () => Date.now() -): BrowserRouteGuestPopupGesture { +): BrowserGuestInputGesture { let lastGestureAt: number | null = null - const onInputEvent = (_event: unknown, input: { type?: string }): void => { + const onInputEvent = (_event: Electron.Event, input: Electron.InputEvent): void => { if (typeof input?.type === 'string' && ACTIVATING_INPUT_TYPES.has(input.type)) { lastGestureAt = now() } } let attached = false try { - guest.on('input-event', onInputEvent as never) + guest.on('input-event', onInputEvent) attached = true } catch { // Fail closed: an unobservable input stream never counts as a gesture. @@ -47,7 +47,7 @@ export function trackBrowserRouteGuestPopupGesture( return ( attached && observedAt !== null && - now() - observedAt <= BROWSER_ROUTE_GUEST_POPUP_GESTURE_WINDOW_MS + now() - observedAt <= BROWSER_GUEST_INPUT_GESTURE_WINDOW_MS ) }, dispose: () => { @@ -57,7 +57,7 @@ export function trackBrowserRouteGuestPopupGesture( } attached = false try { - guest.off('input-event', onInputEvent as never) + guest.off('input-event', onInputEvent) } catch {} } } diff --git a/src/main/browser/browser-manager-guest-lifecycle.test.ts b/src/main/browser/browser-manager-guest-lifecycle.test.ts index 425d1fe59f6..e13801282f5 100644 --- a/src/main/browser/browser-manager-guest-lifecycle.test.ts +++ b/src/main/browser/browser-manager-guest-lifecycle.test.ts @@ -297,8 +297,8 @@ describe('browserManager', () => { expect(browserManager.getGuestWebContentsId('browser-1')).toBeNull() expect(browserManager.getGuestWebContentsId('browser-2')).toBeNull() expect(guestOffMock).toHaveBeenCalled() - expect(unregisteredGuestOff).toHaveBeenCalledWith('dom-ready', expect.any(Function)) - expect(unregisteredGuestOff).toHaveBeenCalledWith('frame-created', expect.any(Function)) + expect(unregisteredGuestOff).toHaveBeenCalledWith('did-create-window', expect.any(Function)) + expect(unregisteredGuestOff).toHaveBeenCalledWith('input-event', expect.any(Function)) }) it('rejects non-webview guest types to prevent privilege escalation', () => { diff --git a/src/main/browser/browser-manager-guest-policy-profile.test.ts b/src/main/browser/browser-manager-guest-policy-profile.test.ts index c5c4c2521fe..a2256a43668 100644 --- a/src/main/browser/browser-manager-guest-policy-profile.test.ts +++ b/src/main/browser/browser-manager-guest-policy-profile.test.ts @@ -47,7 +47,6 @@ type GuestFake = { setBackgroundThrottling: (value: boolean) => void setWebRTCIPHandlingPolicy: (policy: string) => void setWindowOpenHandler: (handler: (details: { url: string; frameName: string }) => unknown) => void - executeJavaScriptInIsolatedWorld: ReturnType debugger: { isAttached: () => boolean attach: ReturnType @@ -79,7 +78,6 @@ function createGuest(id: number, url: string): GuestFake { setWindowOpenHandler: (handler) => { guest.windowOpenHandler = handler }, - executeJavaScriptInIsolatedWorld: vi.fn(async () => undefined), debugger: { isAttached: () => true, attach: vi.fn(), @@ -147,13 +145,12 @@ describe('guest policy profiles', () => { // The presence half of every absence below: a browsing guest observably takes all of it through // the same method, so a profile that fenced nothing — or an attach path that stopped installing // anything at all — cannot pass these by being uniformly empty. - it('gives a browsing guest link routing, popups and auth-identity detach tracking', () => { + it('gives a browsing guest popups, link gestures and auth-identity detach tracking', () => { const guest = createGuest(300, 'https://example.com/') browserManager.attachGuestPolicies(guest as never) - expect(listenerCount(guest, 'dom-ready')).toBe(1) - expect(listenerCount(guest, 'frame-created')).toBe(1) + expect(listenerCount(guest, 'input-event')).toBe(1) expect(listenerCount(guest, 'did-create-window')).toBe(1) expect(guest.debugger.on).toHaveBeenCalledWith('detach', expect.any(Function)) // Why: a plain browsing tab must never attach a debugger; Cloudflare treats CDP as a bot signal. @@ -165,12 +162,10 @@ describe('guest policy profiles', () => { it('gives a workspace-document guest none of it', () => { const { guest } = attachPreviewGuest() - expect(listenerCount(guest, 'dom-ready')).toBe(0) - expect(listenerCount(guest, 'frame-created')).toBe(0) + expect(listenerCount(guest, 'input-event')).toBe(0) expect(listenerCount(guest, 'did-create-window')).toBe(0) expect(guest.debugger.on).not.toHaveBeenCalled() expect(guest.debugger.sendCommand).not.toHaveBeenCalled() - expect(guest.executeJavaScriptInIsolatedWorld).not.toHaveBeenCalled() }) it('holds a workspace-document guest inside the grant it is showing', () => { @@ -204,7 +199,7 @@ describe('guest policy profiles', () => { // The manager's own bookkeeping was reached too: a second attach is refused while the id is // still marked policy-attached, and accepted once its teardown has run. browserManager.attachGuestPolicies(guest as never) - expect(listenerCount(guest, 'dom-ready')).toBe(1) + expect(listenerCount(guest, 'did-create-window')).toBe(1) }) // The seam the whole split rests on: one public door answers for both halves, and each page id @@ -252,7 +247,7 @@ describe('guest policy profiles', () => { browserManager.attachGuestPolicies(guest as never) - expect(listenerCount(guest, 'dom-ready')).toBe(0) + expect(listenerCount(guest, 'did-create-window')).toBe(0) expect(listenerCount(guest, 'will-navigate')).toBe(1) }) }) diff --git a/src/main/browser/browser-manager-guest-policy.ts b/src/main/browser/browser-manager-guest-policy.ts index 270df87a9a8..742cf437045 100644 --- a/src/main/browser/browser-manager-guest-policy.ts +++ b/src/main/browser/browser-manager-guest-policy.ts @@ -29,7 +29,7 @@ export abstract class BrowserManagerGuestPolicy extends BrowserManagerGuestClean const disposeDetachTracking = this.trackDebuggerDetachForCdpOverrides(guest) // Why: disable throttling so background screenshots still get frames; else the compositor stalls and capture returns empty. guest.setBackgroundThrottling(false) - const disposePopupPolicy = this.installGuestPopupPolicy(guest, !inheritedOwnerContext) + const disposePopupPolicy = this.installGuestPopupPolicy(guest) const disposeNavigationPolicy = this.installGuestNavigationPolicy(guest) // Why: store cleanup so unregisterGuest can drop these listeners on teardown and let the WebContents wrapper GC. diff --git a/src/main/browser/browser-manager-guest-popup-policy.ts b/src/main/browser/browser-manager-guest-popup-policy.ts index 70c3ef5abb6..85f22315e34 100644 --- a/src/main/browser/browser-manager-guest-popup-policy.ts +++ b/src/main/browser/browser-manager-guest-popup-policy.ts @@ -1,179 +1,38 @@ import { shell } from 'electron' -import { randomUUID } from 'node:crypto' import { ORCA_BROWSER_BLANK_URL } from '../../shared/constants' import { normalizeBrowserNavigationUrl, normalizeExternalBrowserUrl, redactKagiSessionToken } from '../../shared/browser-url' -import { - BROWSER_CLICKED_LINK_ROUTING_WORLD_ID, - buildBrowserClickedLinkRoutingScript, - buildBrowserIframeClickedLinkRoutingScript, - type BrowserClickedLinkFrameNames -} from './browser-clicked-link-routing' +import { trackBrowserGuestInputGesture } from './browser-guest-input-gesture' import { isNewBrowserTabPopupIntent } from './browser-popup-new-tab-intent' import { SAFE_POPUP_WINDOW_OPTIONS, safeOrigin } from './browser-manager-types' import type { PopupChildWindowOptions } from './popup-origin-bar-window' import { BrowserManagerNavigation } from './browser-manager-navigation' export abstract class BrowserManagerGuestPopupPolicy extends BrowserManagerNavigation { - protected installGuestPopupPolicy( - guest: Electron.WebContents, - routeClickedLinks: boolean - ): () => void { - // OAuth child windows keep native link behavior. - const clickedLinkFrameNames: BrowserClickedLinkFrameNames | null = routeClickedLinks - ? { - foreground: `__orca_clicked_link_foreground_${randomUUID()}`, - background: `__orca_clicked_link_background_${randomUUID()}` - } - : null - let clickedLinkRoutingActive = routeClickedLinks - const installClickedLinkRouting = (): void => { - if (!clickedLinkRoutingActive || !clickedLinkFrameNames || guest.isDestroyed()) { - return - } - // Why: an isolated-world listener labels real anchor clicks without exposing the frame name to page scripts. - void guest - .executeJavaScriptInIsolatedWorld( - BROWSER_CLICKED_LINK_ROUTING_WORLD_ID, - [ - { - // Why: mobile emulation spoofs the UA as iOS, so use the real host platform from main for modifier routing. - code: buildBrowserClickedLinkRoutingScript( - clickedLinkFrameNames.foreground, - clickedLinkFrameNames.background, - process.platform === 'darwin' - ) - } - ], - false - ) - .catch(() => {}) - } - if (clickedLinkFrameNames) { - guest.on('dom-ready', installClickedLinkRouting) - } - const pendingIframeRoutingInstalls = new Map void>() - const iframeFrameNamesByFrame = new Map() - const iframeRoutingByFrameName = new Map< - string, - { frame: Electron.WebFrameMain; activate: boolean } - >() - const clearIframeFrameName = (frame: Electron.WebFrameMain): void => { - const names = iframeFrameNamesByFrame.get(frame) - if (!names) { - return - } - iframeFrameNamesByFrame.delete(frame) - for (const name of [names.foreground, names.background]) { - iframeRoutingByFrameName.delete(name) - } - } - const installIframeClickedLinkRouting = (frame: Electron.WebFrameMain): void => { - clearIframeFrameName(frame) - if (!clickedLinkRoutingActive || frame.isDestroyed()) { - return - } - const foregroundName = `__orca_clicked_link_iframe_foreground_${randomUUID()}` - const backgroundName = `__orca_clicked_link_iframe_background_${randomUUID()}` - iframeFrameNamesByFrame.set(frame, { - foreground: foregroundName, - background: backgroundName - }) - iframeRoutingByFrameName.set(foregroundName, { frame, activate: true }) - iframeRoutingByFrameName.set(backgroundName, { frame, activate: false }) - // Why: child-frame tokens live in the page world, so consume after one trusted click and replace before the next. - void frame - .executeJavaScript( - buildBrowserIframeClickedLinkRoutingScript( - foregroundName, - backgroundName, - process.platform === 'darwin' - ), - false - ) - .catch(() => { - if (iframeFrameNamesByFrame.get(frame)?.foreground === foregroundName) { - clearIframeFrameName(frame) - } - }) - } - const handleFrameCreated = ( - _event: Electron.Event, - { frame }: Electron.FrameCreatedDetails - ): void => { - if (!clickedLinkFrameNames || !frame || frame.parent === null) { - return - } - for (const knownFrame of iframeFrameNamesByFrame.keys()) { - if (knownFrame.isDestroyed()) { - clearIframeFrameName(knownFrame) - } - } - const installAfterDomReady = (): void => { - pendingIframeRoutingInstalls.delete(frame) - installIframeClickedLinkRouting(frame) - } - pendingIframeRoutingInstalls.set(frame, installAfterDomReady) - frame.once('dom-ready', installAfterDomReady) - } - if (clickedLinkFrameNames) { - guest.on('frame-created', handleFrameCreated) - } + protected installGuestPopupPolicy(guest: Electron.WebContents): () => void { + // Why: a fresh input marks a click, which bypasses the page-initiated tab budget. + const gesture = trackBrowserGuestInputGesture(guest) const handleDidCreateWindow = (window: Electron.BrowserWindow): void => { // Why: popup descendants inherit the opener's owner context but must not replace its primary registration. this.attachGuestPolicies(window.webContents, this.resolvePopupOwnerContext(guest.id)) } guest.on('did-create-window', handleDidCreateWindow) - guest.setWindowOpenHandler(({ url, frameName, disposition, features }) => { + guest.setWindowOpenHandler(({ url, frameName, disposition }) => { + // Why: as in Chrome, every open spends the click, so a popup's click cannot fund a later tab. + const clicked = gesture.consume() const ownerContext = this.resolvePopupOwnerContext(guest.id) const browserTabId = ownerContext?.browserTabId ?? null const browserUrl = normalizeBrowserNavigationUrl(url) const externalUrl = normalizeExternalBrowserUrl(url) - const expectedClickedLinkFrameNames = clickedLinkRoutingActive ? clickedLinkFrameNames : null - const iframeRouting = frameName ? iframeRoutingByFrameName.get(frameName) : undefined - let clickedLinkActivate: boolean | null = null - if (expectedClickedLinkFrameNames && frameName === expectedClickedLinkFrameNames.foreground) { - clickedLinkActivate = true - } else if ( - expectedClickedLinkFrameNames && - frameName === expectedClickedLinkFrameNames.background - ) { - clickedLinkActivate = false - } else if (iframeRouting) { - clickedLinkActivate = iframeRouting.activate - clearIframeFrameName(iframeRouting.frame) - queueMicrotask(() => installIframeClickedLinkRouting(iframeRouting.frame)) - } - - if (clickedLinkActivate !== null) { - if ( - browserTabId && - browserUrl && - this.openLinkInOrcaTab(browserTabId, browserUrl, clickedLinkActivate) - ) { - this.forwardOrQueuePopupEvent(guest.id, { - origin: safeOrigin(browserUrl), - action: 'opened-in-orca' - }) - } - // Why: a recognized gesture must never fall through to a native popup if its renderer vanished mid-click. - return { action: 'deny' } - } - - // Why: an unnamed, featureless window.open() is Chromium's own new-tab shape, so an Orca tab is - // the honest presentation; a floating origin-bar window is not. Opener-dependent shapes are - // excluded by isNewBrowserTabPopupIntent and still get a real child window below. - if ( - ownerContext && - externalUrl && - isNewBrowserTabPopupIntent({ frameName, disposition, features }) - ) { + // Why: one rule for every link and window.open; opener-dependent shapes are excluded by + // isNewBrowserTabPopupIntent and still get a real child window below. + if (ownerContext && externalUrl && isNewBrowserTabPopupIntent({ frameName, disposition })) { // Why: one activation lets a page loop window.open, and each routed tab persists into - // workspace session state, so it survives the quit that used to clear popup windows. - if (!this.tryConsumePageInitiatedTab(ownerContext.rootGuestWebContentsId)) { + // workspace session state, so only opens beyond the observed clicks draw on the budget. + if (!clicked && !this.tryConsumePageInitiatedTab(ownerContext.rootGuestWebContentsId)) { this.forwardOrQueuePopupEvent(guest.id, { origin: safeOrigin(externalUrl), action: 'blocked' @@ -225,21 +84,9 @@ export abstract class BrowserManagerGuestPopupPolicy extends BrowserManagerNavig }) return () => { - clickedLinkRoutingActive = false + gesture.dispose() try { guest.off('did-create-window', handleDidCreateWindow) - if (clickedLinkFrameNames) { - guest.off('dom-ready', installClickedLinkRouting) - guest.off('frame-created', handleFrameCreated) - for (const [frame, install] of pendingIframeRoutingInstalls) { - if (!frame.isDestroyed()) { - frame.off('dom-ready', install) - } - } - pendingIframeRoutingInstalls.clear() - iframeFrameNamesByFrame.clear() - iframeRoutingByFrameName.clear() - } } catch { // guest may already be destroyed } diff --git a/src/main/browser/browser-manager-popup-routing.test.ts b/src/main/browser/browser-manager-popup-routing.test.ts index c0247796d09..a5a6bd39086 100644 --- a/src/main/browser/browser-manager-popup-routing.test.ts +++ b/src/main/browser/browser-manager-popup-routing.test.ts @@ -40,7 +40,10 @@ vi.mock('./popup-origin-bar-window', () => ({ })) import { browserManager } from './browser-manager' -import { MAX_PAGE_INITIATED_TABS_PER_WINDOW } from './browser-page-initiated-tab-budget' +import { + MAX_PAGE_INITIATED_TABS_PER_WINDOW, + PAGE_INITIATED_TAB_WINDOW_MS +} from './browser-page-initiated-tab-budget' import { rendererWebContentsId, resetBrowserManagerMocks, @@ -171,9 +174,9 @@ describe('browserManager', () => { it('keeps opener-dependent window.open popups in-app for every disposition', () => { // Regression guard for the reverted #8332: gating the allow on // disposition === 'new-window' silently broke featureless window.open() - // OAuth flows, whose returned handle must stay live. A named target, a - // features string, and a blank URL each mark such a flow, so all three keep - // a real child window no matter which disposition Chromium reports. + // OAuth flows, whose returned handle must stay live. A named target that + // keeps its opener and a blank URL each mark such a flow, so both keep a + // real child window no matter which disposition Chromium reports. const guest = { id: 140, isDestroyed: vi.fn(() => false), @@ -201,7 +204,7 @@ describe('browserManager', () => { }) => { action: 'allow' | 'deny' } const openerDependentOpens = [ { url: 'https://sso.example.com/auth', frameName: 'ssoWindow', features: '' }, - { url: 'https://sso.example.com/auth', frameName: '', features: 'width=500,height=600' }, + { url: 'https://sso.example.com/auth', frameName: 'ssoWindow', features: 'noopener=0' }, { url: 'about:blank', frameName: '', features: '' } ] for (const disposition of ['foreground-tab', 'background-tab', 'new-window']) { @@ -209,6 +212,17 @@ describe('browserManager', () => { expect(handler({ ...open, disposition })).toMatchObject({ action: 'allow' }) } } + // Chromium reports size/position features as a popup, with or without an opener. + for (const features of ['width=500,height=600', 'noopener,width=500,height=600']) { + expect( + handler({ + url: 'https://sso.example.com/auth', + frameName: '', + features, + disposition: 'new-window' + }) + ).toMatchObject({ action: 'allow' }) + } expect(shellOpenExternalMock).not.toHaveBeenCalled() }) @@ -288,6 +302,149 @@ describe('browserManager', () => { expect(shellOpenExternalMock).not.toHaveBeenCalled() }) + it('opens unnamed window.open with only opener features as a tab, as Chrome does', () => { + const rendererSendMock = vi.fn() + const guest = { + id: 143, + isDestroyed: vi.fn(() => false), + getType: vi.fn(() => 'webview'), + setBackgroundThrottling: guestSetBackgroundThrottlingMock, + setWindowOpenHandler: guestSetWindowOpenHandlerMock, + on: guestOnMock, + off: guestOffMock, + openDevTools: guestOpenDevToolsMock + } + webContentsFromIdMock.mockImplementation((id: number) => { + if (id === guest.id) { + return guest + } + if (id === rendererWebContentsId) { + return { isDestroyed: vi.fn(() => false), send: rendererSendMock } + } + return null + }) + + browserManager.attachGuestPolicies(guest as never) + browserManager.registerGuest({ + browserPageId: 'browser-1', + webContentsId: guest.id, + rendererWebContentsId + }) + + const handler = guestSetWindowOpenHandlerMock.mock.calls[0][0] as (details: { + url: string + frameName: string + features: string + disposition: string + }) => { action: 'allow' | 'deny' } + // Chromium reports these as tabs: noopener and noreferrer are not window features. + const openerlessOpens = [ + { frameName: '', features: 'noopener,noreferrer' }, + { frameName: '', features: 'noopener' }, + { frameName: '', features: 'noopener=0' } + ] + for (const open of openerlessOpens) { + expect( + handler({ url: 'https://docs.example.com/guide', disposition: 'foreground-tab', ...open }) + ).toEqual({ action: 'deny' }) + } + expect( + rendererSendMock.mock.calls.filter(([channel]) => channel === 'browser:open-link-in-orca-tab') + ).toHaveLength(openerlessOpens.length) + expect(rendererSendMock).toHaveBeenCalledWith('browser:open-link-in-orca-tab', { + browserPageId: 'browser-1', + url: 'https://docs.example.com/guide' + }) + expect(openPopupWithOriginBarMock).not.toHaveBeenCalled() + expect(shellOpenExternalMock).not.toHaveBeenCalled() + }) + + it('opens a tab for every clicked link but caps scripted opens', () => { + vi.useFakeTimers() + const rendererSendMock = vi.fn() + const guest = { + id: 144, + isDestroyed: vi.fn(() => false), + getType: vi.fn(() => 'webview'), + setBackgroundThrottling: guestSetBackgroundThrottlingMock, + setWindowOpenHandler: guestSetWindowOpenHandlerMock, + on: guestOnMock, + off: guestOffMock, + openDevTools: guestOpenDevToolsMock + } + webContentsFromIdMock.mockImplementation((id: number) => { + if (id === guest.id) { + return guest + } + if (id === rendererWebContentsId) { + return { isDestroyed: vi.fn(() => false), send: rendererSendMock } + } + return null + }) + + browserManager.attachGuestPolicies(guest as never) + browserManager.registerGuest({ + browserPageId: 'browser-1', + webContentsId: guest.id, + rendererWebContentsId + }) + + const handler = guestSetWindowOpenHandlerMock.mock.calls[0][0] as (details: { + url: string + frameName: string + features: string + disposition: string + }) => { action: 'allow' | 'deny' } + const inputEventHandler = guestOnMock.mock.calls.find(([event]) => event === 'input-event')?.[1] + const open = (index: number): void => { + handler({ + url: `https://docs.example.com/${index}`, + frameName: '', + features: '', + disposition: 'background-tab' + }) + } + const routedTabCount = (): number => + rendererSendMock.mock.calls.filter(([channel]) => channel === 'browser:open-link-in-orca-tab') + .length + + const clicks = MAX_PAGE_INITIATED_TABS_PER_WINDOW * 2 + for (let index = 0; index < clicks; index++) { + inputEventHandler({}, { type: 'mouseUp' }) + open(index) + } + expect(routedTabCount()).toBe(clicks) + + // One click cannot be replayed: a loop after it draws on the budget. + inputEventHandler({}, { type: 'mouseUp' }) + for (let index = 0; index < MAX_PAGE_INITIATED_TABS_PER_WINDOW * 2; index++) { + open(clicks + index) + } + expect(routedTabCount()).toBe(clicks + 1 + MAX_PAGE_INITIATED_TABS_PER_WINDOW) + + // A click spent on a popup window cannot fund a later scripted tab. + vi.advanceTimersByTime(PAGE_INITIATED_TAB_WINDOW_MS) + inputEventHandler({}, { type: 'mouseUp' }) + expect( + handler({ + url: 'https://sso.example.com/auth', + frameName: 'sso', + features: 'width=500,height=600', + disposition: 'new-window' + }) + ).toMatchObject({ action: 'allow' }) + const tabsBeforeScriptedLoop = routedTabCount() + for (let index = 0; index < MAX_PAGE_INITIATED_TABS_PER_WINDOW * 2; index++) { + open(clicks * 2 + index) + } + expect(routedTabCount()).toBe(tabsBeforeScriptedLoop + MAX_PAGE_INITIATED_TABS_PER_WINDOW) + expect(rendererSendMock).toHaveBeenCalledWith('browser:popup', { + browserPageId: 'browser-1', + origin: 'https://docs.example.com', + action: 'blocked' + }) + }) + it('shares the page-initiated tab budget across the whole opener popup tree', () => { const rendererSendMock = vi.fn() const guest = { @@ -384,182 +541,6 @@ describe('browserManager', () => { expect(openPopupWithOriginBarMock).toHaveBeenCalledTimes(childHandlers.length) }) - it('keeps plain links current and routes explicit new-tab gestures to Orca tabs', async () => { - const rendererSendMock = vi.fn() - const executeJavaScriptInIsolatedWorldMock = vi.fn().mockResolvedValue(undefined) - const guest = { - id: 141, - isDestroyed: vi.fn(() => false), - getType: vi.fn(() => 'webview'), - setBackgroundThrottling: guestSetBackgroundThrottlingMock, - setWindowOpenHandler: guestSetWindowOpenHandlerMock, - on: guestOnMock, - off: guestOffMock, - openDevTools: guestOpenDevToolsMock, - executeJavaScriptInIsolatedWorld: executeJavaScriptInIsolatedWorldMock - } - webContentsFromIdMock.mockImplementation((id: number) => { - if (id === guest.id) { - return guest - } - if (id === rendererWebContentsId) { - return { isDestroyed: vi.fn(() => false), send: rendererSendMock } - } - return null - }) - - browserManager.attachGuestPolicies(guest as never) - browserManager.registerGuest({ - browserPageId: 'browser-1', - webContentsId: guest.id, - rendererWebContentsId - }) - - const domReadyHandler = guestOnMock.mock.calls.find(([event]) => event === 'dom-ready')?.[1] as - | (() => void) - | undefined - domReadyHandler?.() - await vi.waitFor(() => expect(executeJavaScriptInIsolatedWorldMock).toHaveBeenCalledTimes(1)) - - const script = executeJavaScriptInIsolatedWorldMock.mock.calls[0][1][0].code as string - const clickedLinkFrameNames = { - foreground: script.match(/__orca_clicked_link_foreground_[0-9a-f-]+/)?.[0], - background: script.match(/__orca_clicked_link_background_[0-9a-f-]+/)?.[0] - } - if (!clickedLinkFrameNames.foreground || !clickedLinkFrameNames.background) { - throw new Error('Expected private clicked-link frame names') - } - expect(clickedLinkFrameNames.foreground).toMatch(/^__orca_clicked_link_foreground_/) - expect(executeJavaScriptInIsolatedWorldMock).toHaveBeenCalledWith( - expect.any(Number), - [ - expect.objectContaining({ - code: expect.stringContaining( - `${JSON.stringify(clickedLinkFrameNames.foreground)},${JSON.stringify(clickedLinkFrameNames.background)},${process.platform === 'darwin'})` - ) - }) - ], - false - ) - - const handler = guestSetWindowOpenHandlerMock.mock.calls[0][0] as (details: { - url: string - frameName: string - }) => { action: 'allow' | 'deny' } - expect( - handler({ - url: 'https://docs.example.com/guide', - frameName: clickedLinkFrameNames.background - }) - ).toEqual({ action: 'deny' }) - - expect(rendererSendMock).toHaveBeenCalledWith('browser:open-link-in-orca-tab', { - browserPageId: 'browser-1', - url: 'https://docs.example.com/guide', - activate: false - }) - expect(rendererSendMock).toHaveBeenCalledWith('browser:popup', { - browserPageId: 'browser-1', - origin: 'https://docs.example.com', - action: 'opened-in-orca' - }) - expect(openPopupWithOriginBarMock).not.toHaveBeenCalled() - expect(shellOpenExternalMock).not.toHaveBeenCalled() - }) - - it('routes child-frame gestures with one-use tokens', async () => { - const rendererSendMock = vi.fn() - const executeJavaScriptMock = vi.fn().mockResolvedValue(undefined) - const frameOnceMock = vi.fn() - const frame = { - parent: {}, - isDestroyed: vi.fn(() => false), - executeJavaScript: executeJavaScriptMock, - once: frameOnceMock, - off: vi.fn() - } - const guest = { - id: 142, - isDestroyed: vi.fn(() => false), - getType: vi.fn(() => 'webview'), - setBackgroundThrottling: guestSetBackgroundThrottlingMock, - setWindowOpenHandler: guestSetWindowOpenHandlerMock, - on: guestOnMock, - off: guestOffMock, - openDevTools: guestOpenDevToolsMock, - executeJavaScriptInIsolatedWorld: vi.fn().mockResolvedValue(undefined) - } - webContentsFromIdMock.mockImplementation((id: number) => { - if (id === guest.id) { - return guest - } - if (id === rendererWebContentsId) { - return { isDestroyed: vi.fn(() => false), send: rendererSendMock } - } - return null - }) - - browserManager.attachGuestPolicies(guest as never) - browserManager.registerGuest({ - browserPageId: 'browser-frame', - webContentsId: guest.id, - rendererWebContentsId - }) - const frameCreatedHandler = guestOnMock.mock.calls.find( - ([event]) => event === 'frame-created' - )?.[1] as ((event: Electron.Event, details: Electron.FrameCreatedDetails) => void) | undefined - frameCreatedHandler?.({} as Electron.Event, { frame } as never) - const frameDomReadyHandler = frameOnceMock.mock.calls.find( - ([event]) => event === 'dom-ready' - )?.[1] as (() => void) | undefined - frameDomReadyHandler?.() - - await vi.waitFor(() => expect(executeJavaScriptMock).toHaveBeenCalledTimes(1)) - const firstScript = executeJavaScriptMock.mock.calls[0][0] as string - const foregroundFrameName = firstScript.match( - /__orca_clicked_link_iframe_foreground_[0-9a-f-]+/ - )?.[0] - const backgroundFrameName = firstScript.match( - /__orca_clicked_link_iframe_background_[0-9a-f-]+/ - )?.[0] - if (!foregroundFrameName) { - throw new Error('Expected a private child-frame routing token') - } - if (!backgroundFrameName) { - throw new Error('Expected a private background child-frame routing token') - } - expect(firstScript).toContain('installBrowserIframeClickedLinkRouting') - expect(executeJavaScriptMock).toHaveBeenCalledWith(firstScript, false) - - const popupHandler = guestSetWindowOpenHandlerMock.mock.calls[0][0] as (details: { - url: string - frameName: string - }) => { action: string } - expect( - popupHandler({ - url: 'https://docs.example.com/from-frame', - frameName: backgroundFrameName - }) - ).toEqual({ action: 'deny' }) - expect(rendererSendMock).toHaveBeenCalledWith('browser:open-link-in-orca-tab', { - browserPageId: 'browser-frame', - url: 'https://docs.example.com/from-frame', - activate: false - }) - expect( - popupHandler({ - url: 'https://docs.example.com/from-frame-sibling', - frameName: foregroundFrameName - }) - ).toMatchObject({ action: 'allow' }) - expect(rendererSendMock).toHaveBeenCalledTimes(2) - - await vi.waitFor(() => expect(executeJavaScriptMock).toHaveBeenCalledTimes(2)) - const secondScript = executeJavaScriptMock.mock.calls[1][0] as string - expect(secondScript).not.toContain(foregroundFrameName) - expect(secondScript).not.toContain(backgroundFrameName) - }) - it('hosts allowed popups in an origin-bar window with inherited guest policies', () => { const rendererSendMock = vi.fn() const guestOnceMock = vi.fn() diff --git a/src/main/browser/browser-manager-types.ts b/src/main/browser/browser-manager-types.ts index 302cf6468ed..d241de9d712 100644 --- a/src/main/browser/browser-manager-types.ts +++ b/src/main/browser/browser-manager-types.ts @@ -46,7 +46,7 @@ export type PopupOwnerContext = { } /** - * What a guest is allowed to be. A browsing guest is the web — popups, clicked-link routing and + * What a guest is allowed to be. A browsing guest is the web — popups, link gestures and * auth-identity tracking all apply. A workspace-document guest renders one granted document and gets none * of that; `host` is the renderer that minted its grant, and the only sink for what it reports. */ diff --git a/src/main/browser/browser-popup-new-tab-intent.test.ts b/src/main/browser/browser-popup-new-tab-intent.test.ts new file mode 100644 index 00000000000..73098c0ed79 --- /dev/null +++ b/src/main/browser/browser-popup-new-tab-intent.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from 'vitest' +import { isNewBrowserTabPopupIntent } from './browser-popup-new-tab-intent' + +describe('isNewBrowserTabPopupIntent', () => { + it.each([ + // target=_blank, window.open(url), and noopener/noreferrer opens all arrive unnamed as a tab. + { frameName: '', disposition: 'foreground-tab', tab: true }, + // Cmd/Ctrl-click and middle-click. + { frameName: '', disposition: 'background-tab', tab: true }, + // Size/position features and Shift-click. + { frameName: '', disposition: 'new-window', tab: false }, + // A named open may use the returned handle, as OAuth does. + { frameName: 'oauth', disposition: 'foreground-tab', tab: false }, + { frameName: 'oauth', disposition: 'new-window', tab: false }, + { frameName: '', disposition: 'other', tab: false } + ])('$frameName/$disposition → tab: $tab', ({ frameName, disposition, tab }) => { + expect(isNewBrowserTabPopupIntent({ frameName, disposition })).toBe(tab) + }) +}) diff --git a/src/main/browser/browser-popup-new-tab-intent.ts b/src/main/browser/browser-popup-new-tab-intent.ts index 316614b4aab..da14996ddc5 100644 --- a/src/main/browser/browser-popup-new-tab-intent.ts +++ b/src/main/browser/browser-popup-new-tab-intent.ts @@ -1,17 +1,18 @@ /** - * Whether a `window.open()` asks for a new tab rather than a popup window. + * Whether a new-window request from a page should open an Orca tab rather than a popup window. * - * Orca answers a tab by denying, which hands the page `null`, so named and featured opens — whose - * flow may use that handle — stay popups. + * Chromium's disposition already carries Chrome's answer: new-tab links, Cmd/Ctrl/middle clicks, and + * `window.open` without window features (`noopener` and `noreferrer` are not features) arrive as a + * tab, while size/position features and Shift-click arrive as `new-window`. Orca answers a tab by + * denying, which hands the page `null`, so a named open — whose flow may use that handle, as OAuth + * does — stays a child window. */ export function isNewBrowserTabPopupIntent(details: { frameName: string disposition: string - features: string }): boolean { return ( details.frameName === '' && - details.features.trim() === '' && (details.disposition === 'foreground-tab' || details.disposition === 'background-tab') ) } diff --git a/src/main/browser/browser-route-guest-popups.ts b/src/main/browser/browser-route-guest-popups.ts index da7d0312375..9b3b77d317d 100644 --- a/src/main/browser/browser-route-guest-popups.ts +++ b/src/main/browser/browser-route-guest-popups.ts @@ -3,9 +3,9 @@ import { normalizeBrowserNavigationUrl } from '../../shared/browser-url' import { ORCA_BROWSER_BLANK_URL } from '../../shared/constants' import { enforceBrowserRouteWebRtcPolicy } from './browser-route-webrtc-policy' import { - trackBrowserRouteGuestPopupGesture, - type BrowserRouteGuestPopupGesture -} from './browser-route-guest-popup-gesture' + trackBrowserGuestInputGesture, + type BrowserGuestInputGesture +} from './browser-guest-input-gesture' import { registerBrowserRouteGuestPopup, releaseBrowserRouteGuestPopup @@ -46,9 +46,9 @@ export function createBrowserRouteGuestPopupController(input: { }): BrowserRouteGuestPopupController { const openPopupWindow = input.dependencies.openPopupWindow ?? openPopupWithOriginBar const popups = new Set() - const gestures = new Set() + const gestures = new Set() const popupWebContentsIds = new Set() - const openerGesture = trackBrowserRouteGuestPopupGesture(input.opener) + const openerGesture = trackBrowserGuestInputGesture(input.opener) gestures.add(openerGesture) // Why only dispose latches: fencing must be reversible, because a reconnected page regains // navigation authority and its OAuth popups have to work again. `isNavigationAllowed` is what @@ -78,7 +78,7 @@ export function createBrowserRouteGuestPopupController(input: { if (!enforceBrowserRouteWebRtcPolicy(contents, () => {})) { return false } - const gesture = trackBrowserRouteGuestPopupGesture(contents) + const gesture = trackBrowserGuestInputGesture(contents) gestures.add(gesture) try { // Descendants inherit the same envelope; nothing about being a popup relaxes it. @@ -141,7 +141,7 @@ export function createBrowserRouteGuestPopupController(input: { } function buildWindowOpenHandler( - gesture: BrowserRouteGuestPopupGesture + gesture: BrowserGuestInputGesture ): (details?: { url?: string }) => Electron.WindowOpenHandlerResponse { return (details) => { const normalized = normalizeRoutePopupUrl(details?.url)