From 07b7e9e68d304d24af068ed8aa6b4bfec0ee718b Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 27 Aug 2026 08:49:02 -0700 Subject: [PATCH] Open target=_blank links and unnamed popups in new Orca tabs (#16720) * feat(browser): open target=_blank links and unnamed popups in new Orca t - Treat target=_blank as a new-tab request matching browser behavior - Route unnamed, featureless window.open() calls to Orca tabs instead of native popups - Add rate limiting to prevent page-initiated tab loops - Inherit session profiles when opening links to maintain isolation boundaries * fix(browser): deny new-tab window.open when renderer is destroyed Move deny action outside conditional to ensure new-tab intents are safely rejected even if renderer vanishes mid-open, preventing native popup fallthrough. Add test coverage and simplify comments. * Share page-initiated tab budget across opener popup tree Prevent pages from bypassing the new-tab rate limit by chaining popup windows. The page-initiated tab quota is now shared by all popups in an opener tree (root + named children), so child windows inherit their root's budget instead of each getting a fresh allocation. --- .../browser-clicked-link-routing.test.ts | 28 +- .../browser/browser-clicked-link-routing.ts | 26 +- .../browser-manager-popup-child.test.ts | 253 ++++++++++++ .../browser-manager-popup-routing.test.ts | 381 +++++++++--------- src/main/browser/browser-manager.ts | 51 ++- .../browser-page-initiated-tab-budget.test.ts | 34 ++ .../browser-page-initiated-tab-budget.ts | 26 ++ .../browser/browser-popup-new-tab-intent.ts | 17 + .../ipc-events/browser-state-ipc-bridge.ts | 15 +- .../browser-state-open-link-profile.test.ts | 104 +++++ tests/e2e/browser-tab.spec.ts | 23 +- 11 files changed, 708 insertions(+), 250 deletions(-) create mode 100644 src/main/browser/browser-manager-popup-child.test.ts create mode 100644 src/main/browser/browser-page-initiated-tab-budget.test.ts create mode 100644 src/main/browser/browser-page-initiated-tab-budget.ts create mode 100644 src/main/browser/browser-popup-new-tab-intent.ts create mode 100644 src/renderer/src/hooks/ipc-events/browser-state-open-link-profile.test.ts diff --git a/src/main/browser/browser-clicked-link-routing.test.ts b/src/main/browser/browser-clicked-link-routing.test.ts index eee5eea35f1..24c26471c3a 100644 --- a/src/main/browser/browser-clicked-link-routing.test.ts +++ b/src/main/browser/browser-clicked-link-routing.test.ts @@ -64,7 +64,7 @@ describe('browser clicked-link routing', () => { vi.restoreAllMocks() }) - it('routes plain target=_blank links back into the current Orca tab', () => { + 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' @@ -73,9 +73,8 @@ describe('browser clicked-link routing', () => { const { event, open } = clickLink(link) - expect(event.defaultPrevented).toBe(false) - expect(open).not.toHaveBeenCalled() - expect(link.getAttribute('target')).toBe('_self') + 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', () => { @@ -127,9 +126,11 @@ describe('browser clicked-link routing', () => { expect(clickLink(cancelled).open).not.toHaveBeenCalled() expect(cancelled.getAttribute('target')).toBe('_blank') - expect(clickLink(rewritten).open).not.toHaveBeenCalled() - expect(rewritten.href).toBe('https://example.com/rewritten') - expect(rewritten.getAttribute('target')).toBe('_self') + // 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', () => { @@ -145,8 +146,10 @@ describe('browser clicked-link routing', () => { document.body.append(svgLink, areaDownload, noHref) installRouting() - expect(clickLink(svgLink).open).not.toHaveBeenCalled() - expect(svgLink.getAttribute('target')).toBe('_self') + expect(clickLink(svgLink).open).toHaveBeenCalledWith( + 'https://example.com/svg', + FOREGROUND_FRAME_NAME + ) expect(clickLink(areaDownload).open).not.toHaveBeenCalled() expect(clickLink(noHref).open).not.toHaveBeenCalled() }) @@ -195,7 +198,7 @@ describe('browser clicked-link routing', () => { expect(script).not.toContain('BrowserClickedLinkRoutingState') }) - it('routes plain iframe target=_blank links into the top-level guest', () => { + 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' @@ -204,9 +207,8 @@ describe('browser clicked-link routing', () => { const { event, open } = clickLink(link) - expect(event.defaultPrevented).toBe(false) - expect(open).not.toHaveBeenCalled() - expect(link.getAttribute('target')).toBe('_top') + 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', () => { diff --git a/src/main/browser/browser-clicked-link-routing.ts b/src/main/browser/browser-clicked-link-routing.ts index 42a8bbf8232..ee6f2fc43d5 100644 --- a/src/main/browser/browser-clicked-link-routing.ts +++ b/src/main/browser/browser-clicked-link-routing.ts @@ -75,8 +75,9 @@ export function installBrowserClickedLinkRouting( const baseTarget = document.querySelector('base[target]')?.getAttribute('target') ?? '' const ownTarget = link.getAttribute('target') const effectiveTarget = (ownTarget === null ? baseTarget : ownTarget).trim().toLowerCase() - const opensNewContext = middleClick || modifierClick - if (!opensNewContext && effectiveTarget !== '_blank') { + // 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 } @@ -96,13 +97,6 @@ export function installBrowserClickedLinkRouting( return } - if (!opensNewContext) { - // Why: changing only the browsing context keeps Chromium's native anchor - // navigation, including referrer policy, attribution, and history. - link.setAttribute('target', '_self') - 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. @@ -118,8 +112,8 @@ export function installBrowserClickedLinkRouting( } /** - * Keeps plain target=_blank clicks inside the top-level guest when Electron's - * isolated-world API cannot target a child frame. + * 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( frameName: string, @@ -161,8 +155,7 @@ export function installBrowserIframeClickedLinkRouting( const baseTarget = document.querySelector('base[target]')?.getAttribute('target') ?? '' const ownTarget = link.getAttribute('target') const effectiveTarget = (ownTarget === null ? baseTarget : ownTarget).trim().toLowerCase() - const opensNewContext = middleClick || modifierClick - if (!opensNewContext && effectiveTarget !== '_blank') { + if (!(middleClick || modifierClick || effectiveTarget === '_blank')) { return } @@ -182,13 +175,6 @@ export function installBrowserIframeClickedLinkRouting( return } - if (!opensNewContext) { - // Why: WebContents isolated worlds only cover the main frame. Rewriting - // to `_top` preserves native anchor semantics without opening a popup. - link.setAttribute('target', '_top') - 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() diff --git a/src/main/browser/browser-manager-popup-child.test.ts b/src/main/browser/browser-manager-popup-child.test.ts new file mode 100644 index 00000000000..bf4caf2b0ba --- /dev/null +++ b/src/main/browser/browser-manager-popup-child.test.ts @@ -0,0 +1,253 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const browserMocks = vi.hoisted(() => ({ + appGetPathMock: vi.fn(() => '/downloads'), + menuBuildFromTemplateMock: vi.fn(), + guestOffMock: vi.fn(), + guestOnMock: vi.fn(), + guestSetBackgroundThrottlingMock: vi.fn(), + guestSetWindowOpenHandlerMock: vi.fn(), + guestOpenDevToolsMock: vi.fn(), + webContentsFromIdMock: vi.fn(), + browserWindowFromWebContentsMock: vi.fn(), + screenGetCursorScreenPointMock: vi.fn(() => ({ x: 0, y: 0 })), + shellOpenExternalMock: vi.fn(), + openPopupWithOriginBarMock: vi.fn() +})) + +vi.mock('electron', () => ({ + app: { getPath: browserMocks.appGetPathMock }, + BrowserWindow: { fromWebContents: browserMocks.browserWindowFromWebContentsMock }, + clipboard: { writeText: vi.fn() }, + shell: { openExternal: browserMocks.shellOpenExternalMock }, + Menu: { buildFromTemplate: browserMocks.menuBuildFromTemplateMock }, + screen: { getCursorScreenPoint: browserMocks.screenGetCursorScreenPointMock }, + webContents: { fromId: browserMocks.webContentsFromIdMock } +})) + +vi.mock('./popup-origin-bar-window', () => ({ + openPopupWithOriginBar: browserMocks.openPopupWithOriginBarMock +})) + +import { browserManager } from './browser-manager' +import { + createDownloadItem, + getDownloadItemEventHandler, + rendererWebContentsId, + resetBrowserManagerMocks, + resetBrowserManagerState +} from './browser-manager-test-harness' + +const { + guestOffMock, + guestOnMock, + guestSetBackgroundThrottlingMock, + guestSetWindowOpenHandlerMock, + guestOpenDevToolsMock, + webContentsFromIdMock, + shellOpenExternalMock +} = browserMocks + +describe('browserManager popup child policies', () => { + beforeEach(() => { + resetBrowserManagerMocks(browserMocks) + resetBrowserManagerState() + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('attaches guest policies to created popup child windows', () => { + const rendererSendMock = vi.fn() + const childSetBackgroundThrottlingMock = vi.fn() + const childSetWindowOpenHandlerMock = vi.fn() + const childOnMock = vi.fn() + const childOffMock = vi.fn() + const childOpenDevToolsMock = vi.fn() + const childGuest = { + id: 4040, + isDestroyed: vi.fn(() => false), + getType: vi.fn(() => 'webview'), + setBackgroundThrottling: childSetBackgroundThrottlingMock, + setWindowOpenHandler: childSetWindowOpenHandlerMock, + on: childOnMock, + off: childOffMock, + openDevTools: childOpenDevToolsMock + } + const guest = { + id: 404, + 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 didCreateWindowHandler = guestOnMock.mock.calls.find( + ([event]) => event === 'did-create-window' + )?.[1] as ((window: { webContents: typeof childGuest }) => void) | undefined + expect(didCreateWindowHandler).toBeTypeOf('function') + + didCreateWindowHandler?.({ webContents: childGuest }) + + expect(childSetBackgroundThrottlingMock).toHaveBeenCalledWith(false) + expect(childSetWindowOpenHandlerMock).toHaveBeenCalledTimes(1) + expect(childOnMock.mock.calls.filter(([event]) => event === 'did-create-window')).toHaveLength( + 1 + ) + expect(childOnMock.mock.calls.filter(([event]) => event === 'will-navigate')).toHaveLength(1) + expect(childOnMock.mock.calls.filter(([event]) => event === 'will-redirect')).toHaveLength(1) + + const childWindowOpenHandler = childSetWindowOpenHandlerMock.mock.calls[0][0] as (details: { + url: string + }) => { action: 'allow' | 'deny' } + expect(childWindowOpenHandler({ url: 'https://identity.example.com/login' })).toMatchObject({ + action: 'allow' + }) + expect(childWindowOpenHandler({ url: 'file:///etc/passwd' })).toEqual({ action: 'deny' }) + expect(rendererSendMock).toHaveBeenCalledWith('browser:popup', { + browserPageId: 'browser-1', + origin: 'null', + action: 'blocked' + }) + browserManager.notifyPermissionDenied({ + guestWebContentsId: childGuest.id, + permission: 'notifications', + rawUrl: 'https://identity.example.com/login' + }) + expect(rendererSendMock).toHaveBeenCalledWith('browser:permission-denied', { + browserPageId: 'browser-1', + permission: 'notifications', + origin: 'https://identity.example.com' + }) + + const childDidFailLoadHandler = childOnMock.mock.calls.find( + ([event]) => event === 'did-fail-load' + )?.[1] as + | (( + event: Electron.Event, + errorCode: number, + errorDescription: string, + validatedURL: string, + isMainFrame: boolean + ) => void) + | undefined + childDidFailLoadHandler?.( + {} as Electron.Event, + -105, + 'Name not resolved', + 'https://identity.example.com/unavailable', + true + ) + expect(rendererSendMock).not.toHaveBeenCalledWith( + 'browser:guest-load-failed', + expect.anything() + ) + + const childDownloadItem = createDownloadItem() + browserManager.handleGuestWillDownload({ + guestWebContentsId: childGuest.id, + item: childDownloadItem + }) + expect(rendererSendMock).toHaveBeenCalledWith( + 'browser:download-requested', + expect.objectContaining({ browserPageId: 'browser-1' }) + ) + const childDownloadDoneHandler = getDownloadItemEventHandler(childDownloadItem, 'once', 'done') + childDownloadDoneHandler?.({} as Electron.Event, 'completed') + + const managerState = browserManager as unknown as { + popupOwnerContextByGuestId: Map + } + expect(managerState.popupOwnerContextByGuestId.has(childGuest.id)).toBe(true) + + const cleanupChildOnMock = vi.fn() + const cleanupChildGuest = { + ...childGuest, + id: 4041, + on: cleanupChildOnMock, + off: vi.fn(), + setBackgroundThrottling: vi.fn(), + setWindowOpenHandler: vi.fn() + } + const childDidCreateWindowHandler = childOnMock.mock.calls.find( + ([event]) => event === 'did-create-window' + )?.[1] as ((window: { webContents: typeof cleanupChildGuest }) => void) | undefined + childDidCreateWindowHandler?.({ webContents: cleanupChildGuest }) + expect(managerState.popupOwnerContextByGuestId.has(cleanupChildGuest.id)).toBe(true) + const cleanupChildWindowOpenHandler = cleanupChildGuest.setWindowOpenHandler.mock + .calls[0][0] as (details: { url: string }) => { action: 'allow' | 'deny' } + expect( + cleanupChildWindowOpenHandler({ url: 'https://identity.example.com/continue' }) + ).toMatchObject({ action: 'allow' }) + const cleanupChildDestroyedHandler = cleanupChildOnMock.mock.calls.find( + ([event]) => event === 'destroyed' + )?.[1] as (() => void) | undefined + cleanupChildDestroyedHandler?.() + expect(managerState.popupOwnerContextByGuestId.has(cleanupChildGuest.id)).toBe(false) + + const replacementGuest = { + ...guest, + id: 405, + on: vi.fn(), + off: vi.fn(), + setBackgroundThrottling: vi.fn(), + setWindowOpenHandler: vi.fn() + } + webContentsFromIdMock.mockImplementation((id: number) => { + if (id === guest.id) { + return guest + } + if (id === replacementGuest.id) { + return replacementGuest + } + if (id === rendererWebContentsId) { + return { isDestroyed: vi.fn(() => false), send: rendererSendMock } + } + return null + }) + browserManager.attachGuestPolicies(replacementGuest as never) + browserManager.registerGuest({ + browserPageId: 'browser-1', + webContentsId: replacementGuest.id, + rendererWebContentsId + }) + + expect(childWindowOpenHandler({ url: 'https://identity.example.com/next' })).toEqual({ + action: 'deny' + }) + expect(shellOpenExternalMock).toHaveBeenCalledWith('https://identity.example.com/next') + expect(managerState.popupOwnerContextByGuestId.has(childGuest.id)).toBe(false) + + const childDestroyedHandler = childOnMock.mock.calls.find( + ([event]) => event === 'destroyed' + )?.[1] as (() => void) | undefined + childDestroyedHandler?.() + expect(managerState.popupOwnerContextByGuestId.has(childGuest.id)).toBe(false) + + browserManager.unregisterAll() + + expect(childOffMock).toHaveBeenCalledWith('did-create-window', expect.any(Function)) + expect(childOffMock).toHaveBeenCalledWith('will-navigate', expect.any(Function)) + expect(childOffMock).toHaveBeenCalledWith('will-redirect', expect.any(Function)) + }) +}) diff --git a/src/main/browser/browser-manager-popup-routing.test.ts b/src/main/browser/browser-manager-popup-routing.test.ts index 89b4d2fe2a8..a3083005016 100644 --- a/src/main/browser/browser-manager-popup-routing.test.ts +++ b/src/main/browser/browser-manager-popup-routing.test.ts @@ -40,9 +40,8 @@ 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 { - createDownloadItem, - getDownloadItemEventHandler, rendererWebContentsId, resetBrowserManagerMocks, resetBrowserManagerState @@ -169,11 +168,12 @@ describe('browserManager', () => { expect(rendererSendMock).not.toHaveBeenCalled() }) - it('keeps featureless window.open popups in-app for every disposition', () => { + 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 (disposition 'foreground-tab'), whose returned handle must - // stay live. Disposition is a UX hint, not a trust signal. + // 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. const guest = { id: 140, isDestroyed: vi.fn(() => false), @@ -199,14 +199,186 @@ describe('browserManager', () => { features: string disposition: string }) => { 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: 'about:blank', frameName: '', features: '' } + ] for (const disposition of ['foreground-tab', 'background-tab', 'new-window']) { - expect( - handler({ url: 'https://sso.example.com/auth', frameName: '', features: '', disposition }) - ).toMatchObject({ action: 'allow' }) + for (const open of openerDependentOpens) { + expect(handler({ ...open, disposition })).toMatchObject({ action: 'allow' }) + } } expect(shellOpenExternalMock).not.toHaveBeenCalled() }) + it('routes unnamed featureless window.open safely, including after renderer destruction', () => { + const rendererSendMock = vi.fn() + let rendererDestroyed = false + const guest = { + id: 142, + 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(() => rendererDestroyed), 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' } + for (const disposition of ['foreground-tab', 'background-tab']) { + expect( + handler({ + url: 'https://docs.example.com/guide', + frameName: '', + features: '', + disposition + }) + ).toEqual({ action: 'deny' }) + } + + expect(rendererSendMock).toHaveBeenCalledWith('browser:open-link-in-orca-tab', { + browserPageId: 'browser-1', + url: 'https://docs.example.com/guide' + }) + 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() + rendererDestroyed = true + expect( + handler({ + url: 'https://docs.example.com/guide', + frameName: '', + features: '', + disposition: 'foreground-tab' + }) + ).toEqual({ action: 'deny' }) + expect(openPopupWithOriginBarMock).not.toHaveBeenCalled() + expect(shellOpenExternalMock).not.toHaveBeenCalled() + }) + + it('shares the page-initiated tab budget across the whole opener popup tree', () => { + const rendererSendMock = vi.fn() + const guest = { + id: 150, + isDestroyed: vi.fn(() => false), + getType: vi.fn(() => 'webview'), + setBackgroundThrottling: guestSetBackgroundThrottlingMock, + setWindowOpenHandler: guestSetWindowOpenHandlerMock, + on: guestOnMock, + once: vi.fn(), + off: guestOffMock, + openDevTools: guestOpenDevToolsMock + } + const guestsById = new Map([[guest.id, guest]]) + webContentsFromIdMock.mockImplementation((id: number) => { + if (id === rendererWebContentsId) { + return { isDestroyed: vi.fn(() => false), send: rendererSendMock } + } + return guestsById.get(id) ?? null + }) + + browserManager.attachGuestPolicies(guest as never) + browserManager.registerGuest({ + browserPageId: 'browser-1', + webContentsId: guest.id, + rendererWebContentsId + }) + + type WindowOpenHandler = (details: { + url: string + frameName: string + features: string + disposition: string + }) => { + action: 'allow' | 'deny' + createWindow?: (options: Record) => unknown + } + const handler = guestSetWindowOpenHandlerMock.mock.calls[0][0] as WindowOpenHandler + + // A named child popup keeps a real child window; each one used to start with a fresh budget. + const openNamedChild = (index: number): WindowOpenHandler => { + const childSetWindowOpenHandlerMock = vi.fn() + const child = { + id: 1500 + index, + isDestroyed: vi.fn(() => false), + getType: vi.fn(() => 'webview'), + setBackgroundThrottling: vi.fn(), + setWindowOpenHandler: childSetWindowOpenHandlerMock, + on: vi.fn(), + once: vi.fn(), + off: vi.fn(), + openDevTools: vi.fn() + } + guestsById.set(child.id, child) + openPopupWithOriginBarMock.mockReturnValueOnce({ + contentWebContents: child, + close: vi.fn(), + onClosed: vi.fn() + }) + const result = handler({ + url: `https://sso.example.com/child-${index}`, + frameName: `child-${index}`, + features: '', + disposition: 'new-window' + }) + expect(result).toMatchObject({ action: 'allow' }) + result.createWindow?.({}) + return childSetWindowOpenHandlerMock.mock.calls[0][0] as WindowOpenHandler + } + + const childHandlers = [openNamedChild(0), openNamedChild(1), openNamedChild(2)] + for (const [childIndex, childHandler] of childHandlers.entries()) { + for (let openIndex = 0; openIndex < MAX_PAGE_INITIATED_TABS_PER_WINDOW; openIndex++) { + expect( + childHandler({ + url: `https://docs.example.com/${childIndex}-${openIndex}`, + frameName: '', + features: '', + disposition: 'foreground-tab' + }) + ).toEqual({ action: 'deny' }) + } + } + + const routedTabs = rendererSendMock.mock.calls.filter( + ([channel]) => channel === 'browser:open-link-in-orca-tab' + ) + expect(routedTabs).toHaveLength(MAX_PAGE_INITIATED_TABS_PER_WINDOW) + expect(rendererSendMock).toHaveBeenCalledWith('browser:popup', { + browserPageId: 'browser-1', + origin: 'https://docs.example.com', + action: 'blocked' + }) + 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) @@ -589,197 +761,4 @@ describe('browserManager', () => { }) ) }) - - it('attaches guest policies to created popup child windows', () => { - const rendererSendMock = vi.fn() - const childSetBackgroundThrottlingMock = vi.fn() - const childSetWindowOpenHandlerMock = vi.fn() - const childOnMock = vi.fn() - const childOffMock = vi.fn() - const childOpenDevToolsMock = vi.fn() - const childGuest = { - id: 4040, - isDestroyed: vi.fn(() => false), - getType: vi.fn(() => 'webview'), - setBackgroundThrottling: childSetBackgroundThrottlingMock, - setWindowOpenHandler: childSetWindowOpenHandlerMock, - on: childOnMock, - off: childOffMock, - openDevTools: childOpenDevToolsMock - } - const guest = { - id: 404, - 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 didCreateWindowHandler = guestOnMock.mock.calls.find( - ([event]) => event === 'did-create-window' - )?.[1] as ((window: { webContents: typeof childGuest }) => void) | undefined - expect(didCreateWindowHandler).toBeTypeOf('function') - - didCreateWindowHandler?.({ webContents: childGuest }) - - expect(childSetBackgroundThrottlingMock).toHaveBeenCalledWith(false) - expect(childSetWindowOpenHandlerMock).toHaveBeenCalledTimes(1) - expect(childOnMock.mock.calls.filter(([event]) => event === 'did-create-window')).toHaveLength( - 1 - ) - expect(childOnMock.mock.calls.filter(([event]) => event === 'will-navigate')).toHaveLength(1) - expect(childOnMock.mock.calls.filter(([event]) => event === 'will-redirect')).toHaveLength(1) - - const childWindowOpenHandler = childSetWindowOpenHandlerMock.mock.calls[0][0] as (details: { - url: string - }) => { action: 'allow' | 'deny' } - expect(childWindowOpenHandler({ url: 'https://identity.example.com/login' })).toMatchObject({ - action: 'allow' - }) - expect(childWindowOpenHandler({ url: 'file:///etc/passwd' })).toEqual({ action: 'deny' }) - expect(rendererSendMock).toHaveBeenCalledWith('browser:popup', { - browserPageId: 'browser-1', - origin: 'null', - action: 'blocked' - }) - browserManager.notifyPermissionDenied({ - guestWebContentsId: childGuest.id, - permission: 'notifications', - rawUrl: 'https://identity.example.com/login' - }) - expect(rendererSendMock).toHaveBeenCalledWith('browser:permission-denied', { - browserPageId: 'browser-1', - permission: 'notifications', - origin: 'https://identity.example.com' - }) - - const childDidFailLoadHandler = childOnMock.mock.calls.find( - ([event]) => event === 'did-fail-load' - )?.[1] as - | (( - event: Electron.Event, - errorCode: number, - errorDescription: string, - validatedURL: string, - isMainFrame: boolean - ) => void) - | undefined - childDidFailLoadHandler?.( - {} as Electron.Event, - -105, - 'Name not resolved', - 'https://identity.example.com/unavailable', - true - ) - expect(rendererSendMock).not.toHaveBeenCalledWith( - 'browser:guest-load-failed', - expect.anything() - ) - - const childDownloadItem = createDownloadItem() - browserManager.handleGuestWillDownload({ - guestWebContentsId: childGuest.id, - item: childDownloadItem - }) - expect(rendererSendMock).toHaveBeenCalledWith( - 'browser:download-requested', - expect.objectContaining({ browserPageId: 'browser-1' }) - ) - const childDownloadDoneHandler = getDownloadItemEventHandler(childDownloadItem, 'once', 'done') - childDownloadDoneHandler?.({} as Electron.Event, 'completed') - - const managerState = browserManager as unknown as { - popupOwnerContextByGuestId: Map - } - expect(managerState.popupOwnerContextByGuestId.has(childGuest.id)).toBe(true) - - const cleanupChildOnMock = vi.fn() - const cleanupChildGuest = { - ...childGuest, - id: 4041, - on: cleanupChildOnMock, - off: vi.fn(), - setBackgroundThrottling: vi.fn(), - setWindowOpenHandler: vi.fn() - } - const childDidCreateWindowHandler = childOnMock.mock.calls.find( - ([event]) => event === 'did-create-window' - )?.[1] as ((window: { webContents: typeof cleanupChildGuest }) => void) | undefined - childDidCreateWindowHandler?.({ webContents: cleanupChildGuest }) - expect(managerState.popupOwnerContextByGuestId.has(cleanupChildGuest.id)).toBe(true) - const cleanupChildWindowOpenHandler = cleanupChildGuest.setWindowOpenHandler.mock - .calls[0][0] as (details: { url: string }) => { action: 'allow' | 'deny' } - expect( - cleanupChildWindowOpenHandler({ url: 'https://identity.example.com/continue' }) - ).toMatchObject({ action: 'allow' }) - const cleanupChildDestroyedHandler = cleanupChildOnMock.mock.calls.find( - ([event]) => event === 'destroyed' - )?.[1] as (() => void) | undefined - cleanupChildDestroyedHandler?.() - expect(managerState.popupOwnerContextByGuestId.has(cleanupChildGuest.id)).toBe(false) - - const replacementGuest = { - ...guest, - id: 405, - on: vi.fn(), - off: vi.fn(), - setBackgroundThrottling: vi.fn(), - setWindowOpenHandler: vi.fn() - } - webContentsFromIdMock.mockImplementation((id: number) => { - if (id === guest.id) { - return guest - } - if (id === replacementGuest.id) { - return replacementGuest - } - if (id === rendererWebContentsId) { - return { isDestroyed: vi.fn(() => false), send: rendererSendMock } - } - return null - }) - browserManager.attachGuestPolicies(replacementGuest as never) - browserManager.registerGuest({ - browserPageId: 'browser-1', - webContentsId: replacementGuest.id, - rendererWebContentsId - }) - - expect(childWindowOpenHandler({ url: 'https://identity.example.com/next' })).toEqual({ - action: 'deny' - }) - expect(shellOpenExternalMock).toHaveBeenCalledWith('https://identity.example.com/next') - expect(managerState.popupOwnerContextByGuestId.has(childGuest.id)).toBe(false) - - const childDestroyedHandler = childOnMock.mock.calls.find( - ([event]) => event === 'destroyed' - )?.[1] as (() => void) | undefined - childDestroyedHandler?.() - expect(managerState.popupOwnerContextByGuestId.has(childGuest.id)).toBe(false) - - browserManager.unregisterAll() - - expect(childOffMock).toHaveBeenCalledWith('did-create-window', expect.any(Function)) - expect(childOffMock).toHaveBeenCalledWith('will-navigate', expect.any(Function)) - expect(childOffMock).toHaveBeenCalledWith('will-redirect', expect.any(Function)) - }) }) diff --git a/src/main/browser/browser-manager.ts b/src/main/browser/browser-manager.ts index 1e04cbdf0cc..7ac64b485b9 100644 --- a/src/main/browser/browser-manager.ts +++ b/src/main/browser/browser-manager.ts @@ -43,6 +43,11 @@ import { buildBrowserClickedLinkRoutingScript, buildBrowserIframeClickedLinkRoutingScript } from './browser-clicked-link-routing' +import { + createPageInitiatedTabBudget, + type PageInitiatedTabBudget +} from './browser-page-initiated-tab-budget' +import { isNewBrowserTabPopupIntent } from './browser-popup-new-tab-intent' import { cleanElectronUserAgent } from './browser-session-ua' import { getBrowserSessionUserAgentMode } from './browser-session-user-agent-mode' import { googleAuthUserAgent, isGoogleAuthUrl } from './browser-google-auth-ua' @@ -233,6 +238,8 @@ export class BrowserManager { // Why: reverse map gives O(1) guest→tab lookups on every mouse/load/permission/popup event. private readonly tabIdByWebContentsId = new Map() private readonly popupOwnerContextByGuestId = new Map() + // Why: keyed by the opener tree's root so named child popups can't each mint a fresh tab quota. + private readonly pageInitiatedTabBudgetByRootGuestId = new Map() // Why: guests are keyed by page id but renderer visibility by workspace id; bridge the mismatch to activate the right tab before capture. private readonly workspaceIdByPageId = new Map() private readonly sessionProfileIdByPageId = new Map() @@ -394,6 +401,16 @@ export class BrowserManager { return null } + /** Shared across the whole opener tree, so a chain of popups draws from one budget. */ + private tryConsumePageInitiatedTab(rootGuestWebContentsId: number): boolean { + let budget = this.pageInitiatedTabBudgetByRootGuestId.get(rootGuestWebContentsId) + if (!budget) { + budget = createPageInitiatedTabBudget() + this.pageInitiatedTabBudgetByRootGuestId.set(rootGuestWebContentsId, budget) + } + return budget.tryConsume(Date.now()) + } + private resolveRendererForBrowserTab(browserTabId: string): Electron.WebContents | null { const rendererWebContentsId = this.rendererWebContentsIdByTabId.get(browserTabId) if (!rendererWebContentsId) { @@ -758,8 +775,9 @@ export class BrowserManager { this.attachGuestPolicies(window.webContents, this.resolvePopupOwnerContext(guest.id)) } guest.on('did-create-window', handleDidCreateWindow) - guest.setWindowOpenHandler(({ url, frameName }) => { - const browserTabId = this.resolveBrowserTabIdForGuestWebContentsId(guest.id) + guest.setWindowOpenHandler(({ url, frameName, disposition, features }) => { + const ownerContext = this.resolvePopupOwnerContext(guest.id) + const browserTabId = ownerContext?.browserTabId ?? null const browserUrl = normalizeBrowserNavigationUrl(url) const externalUrl = normalizeExternalBrowserUrl(url) const expectedClickedLinkFrameName = this.clickedLinkFrameNameByGuestId.get(guest.id) @@ -784,6 +802,33 @@ export class BrowserManager { 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 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)) { + this.forwardOrQueuePopupEvent(guest.id, { + origin: safeOrigin(externalUrl), + action: 'blocked' + }) + return { action: 'deny' } + } + if (this.openLinkInOrcaTab(ownerContext.browserTabId, externalUrl)) { + this.forwardOrQueuePopupEvent(guest.id, { + origin: safeOrigin(externalUrl), + action: 'opened-in-orca' + }) + } + // Why: a recognized new-tab intent must never fall through to a native popup if its renderer vanished mid-open. + return { action: 'deny' } + } + // Why: file URLs are fine for in-pane previews, but must not spawn native child windows targeting local paths. const canOpenAsChild = Boolean(externalUrl || browserUrl === ORCA_BROWSER_BLANK_URL) if (browserTabId && canOpenAsChild) { @@ -1243,6 +1288,7 @@ export class BrowserManager { this.clickedLinkFrameNameByGuestId.delete(guestWebContentsId) this.offscreenGuestIds.delete(guestWebContentsId) this.popupOwnerContextByGuestId.delete(guestWebContentsId) + this.pageInitiatedTabBudgetByRootGuestId.delete(guestWebContentsId) this.authUserAgentOverrideStateByGuestId.delete(guestWebContentsId) this.pendingNavigationByGuestId.delete(guestWebContentsId) // Why: a popup must stop inheriting authorization the moment its owner retires, before Chromium destroys the child. @@ -1444,6 +1490,7 @@ export class BrowserManager { this.clickedLinkFrameNameByGuestId.clear() this.tabIdByWebContentsId.clear() this.popupOwnerContextByGuestId.clear() + this.pageInitiatedTabBudgetByRootGuestId.clear() this.worktreeIdByTabId.clear() this.sessionProfileIdByPageId.clear() this.userAgentModeByPageId.clear() diff --git a/src/main/browser/browser-page-initiated-tab-budget.test.ts b/src/main/browser/browser-page-initiated-tab-budget.test.ts new file mode 100644 index 00000000000..c2dc7618a7f --- /dev/null +++ b/src/main/browser/browser-page-initiated-tab-budget.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from 'vitest' +import { + createPageInitiatedTabBudget, + MAX_PAGE_INITIATED_TABS_PER_WINDOW, + PAGE_INITIATED_TAB_WINDOW_MS +} from './browser-page-initiated-tab-budget' + +describe('page-initiated tab budget', () => { + it('absorbs a window.open loop fired from a single activation', () => { + const budget = createPageInitiatedTabBudget() + const granted = Array.from({ length: 12 }, () => budget.tryConsume(1_000)).filter(Boolean) + + expect(granted).toHaveLength(MAX_PAGE_INITIATED_TABS_PER_WINDOW) + }) + + it('refills once the rolling window has passed, so real browsing is unaffected', () => { + const budget = createPageInitiatedTabBudget() + for (let i = 0; i < MAX_PAGE_INITIATED_TABS_PER_WINDOW; i++) { + expect(budget.tryConsume(1_000)).toBe(true) + } + expect(budget.tryConsume(1_000 + PAGE_INITIATED_TAB_WINDOW_MS - 1)).toBe(false) + expect(budget.tryConsume(1_000 + PAGE_INITIATED_TAB_WINDOW_MS)).toBe(true) + }) + + it('slides rather than resetting, so a paced flood cannot outrun the cap', () => { + const budget = createPageInitiatedTabBudget(2, 1_000) + + expect(budget.tryConsume(0)).toBe(true) + expect(budget.tryConsume(900)).toBe(true) + // The first grant has aged out by 1_000 but the second has not. + expect(budget.tryConsume(1_000)).toBe(true) + expect(budget.tryConsume(1_100)).toBe(false) + }) +}) diff --git a/src/main/browser/browser-page-initiated-tab-budget.ts b/src/main/browser/browser-page-initiated-tab-budget.ts new file mode 100644 index 00000000000..d15a5c9ba4c --- /dev/null +++ b/src/main/browser/browser-page-initiated-tab-budget.ts @@ -0,0 +1,26 @@ +/** Bounded so one hostile click cannot plant an unbounded number of tabs. */ +export const MAX_PAGE_INITIATED_TABS_PER_WINDOW = 4 +export const PAGE_INITIATED_TAB_WINDOW_MS = 2_000 + +export type PageInitiatedTabBudget = { + /** Records the grant when it returns true; call only when the tab is actually being opened. */ + tryConsume: (now: number) => boolean +} + +/** Rolling window, so it absorbs a same-tick `window.open` loop without capping real browsing. */ +export function createPageInitiatedTabBudget( + maxPerWindow = MAX_PAGE_INITIATED_TABS_PER_WINDOW, + windowMs = PAGE_INITIATED_TAB_WINDOW_MS +): PageInitiatedTabBudget { + let grants: number[] = [] + return { + tryConsume: (now) => { + grants = grants.filter((grantedAt) => now - grantedAt < windowMs) + if (grants.length >= maxPerWindow) { + return false + } + grants.push(now) + return true + } + } +} diff --git a/src/main/browser/browser-popup-new-tab-intent.ts b/src/main/browser/browser-popup-new-tab-intent.ts new file mode 100644 index 00000000000..316614b4aab --- /dev/null +++ b/src/main/browser/browser-popup-new-tab-intent.ts @@ -0,0 +1,17 @@ +/** + * Whether a `window.open()` asks for a new 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. + */ +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/renderer/src/hooks/ipc-events/browser-state-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/browser-state-ipc-bridge.ts index 15a632a25e2..4f3450dae85 100644 --- a/src/renderer/src/hooks/ipc-events/browser-state-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/browser-state-ipc-bridge.ts @@ -89,7 +89,20 @@ export function registerBrowserStateIpcBridge( if (!sourcePage || getRuntimeEnvironmentIdForWorktree(store, sourcePage.worktreeId)) { return } - store.createBrowserTab(sourcePage.worktreeId, url, { title: url }) + // Why: the link inherits the opener's cookie jar. Falling back to the default profile would let + // a page in an isolated session hand its links to the default one, silently crossing profiles. + const sourceTab = (store.browserTabsByWorktree[sourcePage.worktreeId] ?? []).find( + (tab) => tab.id === sourcePage.workspaceId + ) + store.createBrowserTab(sourcePage.worktreeId, url, { + title: url, + ...(sourceTab + ? { + sessionProfileId: sourceTab.sessionProfileId, + sessionPartition: sourceTab.sessionPartition + } + : {}) + }) }) ) } diff --git a/src/renderer/src/hooks/ipc-events/browser-state-open-link-profile.test.ts b/src/renderer/src/hooks/ipc-events/browser-state-open-link-profile.test.ts new file mode 100644 index 00000000000..b15a5be158c --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/browser-state-open-link-profile.test.ts @@ -0,0 +1,104 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { createBrowserTabMock, storeState } = vi.hoisted(() => ({ + createBrowserTabMock: vi.fn(), + storeState: { + value: {} as Record + } +})) + +vi.mock('../../store', () => ({ + useAppStore: { getState: () => storeState.value } +})) +vi.mock('@/lib/worktree-runtime-owner', () => ({ + getRuntimeEnvironmentIdForWorktree: () => null +})) +vi.mock('@/components/browser-pane/describe-page/live-browser-url-registry', () => ({ + rememberLiveBrowserUrl: vi.fn() +})) +vi.mock('./browser-automation-bootstrap-lease', () => ({ + acquireBrowserAutomationBootstrapLease: vi.fn() +})) + +import { registerBrowserStateIpcBridge } from './browser-state-ipc-bridge' + +const noopUnsubscribe = (): void => {} + +function captureOpenLinkHandler(): (event: { browserPageId: string; url: string }) => void { + let handler: ((event: { browserPageId: string; url: string }) => void) | null = null + const browserApi = new Proxy( + { + onOpenLinkInOrcaTab: (callback: (event: { browserPageId: string; url: string }) => void) => { + handler = callback + return noopUnsubscribe + } + } as Record, + { + get: (target, property) => + property in target ? target[property as string] : () => noopUnsubscribe + } + ) + const api = new Proxy({ browser: browserApi } as Record, { + get: (target, property) => + property in target + ? target[property as string] + : new Proxy({}, { get: () => () => noopUnsubscribe }) + }) + ;(globalThis as { window?: unknown }).window = { api } + + registerBrowserStateIpcBridge([], () => false) + if (!handler) { + throw new Error('Expected the bridge to subscribe to browser:open-link-in-orca-tab') + } + return handler +} + +describe('link-opened Orca tabs', () => { + beforeEach(() => { + createBrowserTabMock.mockReset() + }) + + it('inherits the opener tab session so an isolated profile cannot leak into the default one', () => { + storeState.value = { + browserPagesByWorkspace: { + 'workspace-1': [{ id: 'page-1', workspaceId: 'workspace-1', worktreeId: 'worktree-1' }] + }, + browserTabsByWorktree: { + 'worktree-1': [ + { + id: 'workspace-1', + sessionProfileId: 'profile-client-a', + sessionPartition: 'persist:orca-browser-session-client-a' + } + ] + }, + createBrowserTab: createBrowserTabMock + } + + captureOpenLinkHandler()({ browserPageId: 'page-1', url: 'https://docs.example.com/guide' }) + + expect(createBrowserTabMock).toHaveBeenCalledWith( + 'worktree-1', + 'https://docs.example.com/guide', + expect.objectContaining({ + sessionProfileId: 'profile-client-a', + sessionPartition: 'persist:orca-browser-session-client-a' + }) + ) + }) + + it('leaves the profile unset when the opener tab is gone, so the user default still applies', () => { + storeState.value = { + browserPagesByWorkspace: { + 'workspace-1': [{ id: 'page-1', workspaceId: 'missing', worktreeId: 'worktree-1' }] + }, + browserTabsByWorktree: {}, + createBrowserTab: createBrowserTabMock + } + + captureOpenLinkHandler()({ browserPageId: 'page-1', url: 'https://docs.example.com/guide' }) + + const options = createBrowserTabMock.mock.calls[0][2] as Record + expect('sessionProfileId' in options).toBe(false) + }) +}) diff --git a/tests/e2e/browser-tab.spec.ts b/tests/e2e/browser-tab.spec.ts index 4988b76ccc2..00075b4af19 100644 --- a/tests/e2e/browser-tab.spec.ts +++ b/tests/e2e/browser-tab.spec.ts @@ -680,7 +680,7 @@ test.describe('Browser Tab', () => { } }) - test('plain links stay current while explicit new-tab gestures activate Orca tabs', async ({ + test('every new-tab link gesture activates an Orca tab and never a native window', async ({ electronApp, orcaPage }) => { @@ -698,23 +698,20 @@ test.describe('Browser Tab', () => { const baseWindowCount = await electronApp.evaluate( ({ BaseWindow }) => BaseWindow.getAllWindows().length ) - const baseTabCount = await orcaPage.locator('[data-tab-id]').count() - await clickBrowserLink(orcaPage, sourceTab!.id, '#external-link') - + // A plain target=_blank click is a new-tab request, in the main frame and in an iframe; + // the source tab must stay put rather than navigate away under it. const sourceTabLocator = orcaPage.locator(`[data-tab-id="${sourceTab!.id}"]`) - await expect(sourceTabLocator).toContainText('Linked destination', { timeout: 10_000 }) - await expect(orcaPage.locator('[data-tab-id]')).toHaveCount(baseTabCount) + await clickBrowserLink(orcaPage, sourceTab!.id, '#external-link') + await expectBrowserTabActive(orcaPage, 'Linked destination') + await expect(sourceTabLocator).toContainText('Source page') + await switchToBrowserTab(orcaPage, worktreeId, sourceTab!.id) - await clickBrowserLink(orcaPage, sourceTab!.id, '#return-link') - await expect(sourceTabLocator).toContainText('Source page', { timeout: 10_000 }) await clickBrowserLink(orcaPage, sourceTab!.id, '#frame-link', { frameSelector: '#link-frame' }) - await expect(sourceTabLocator).toContainText('Frame destination', { timeout: 10_000 }) - await expect(orcaPage.locator('[data-tab-id]')).toHaveCount(baseTabCount) - - await clickBrowserLink(orcaPage, sourceTab!.id, '#return-link') - await expect(sourceTabLocator).toContainText('Source page', { timeout: 10_000 }) + await expectBrowserTabActive(orcaPage, 'Frame destination') + await expect(sourceTabLocator).toContainText('Source page') + await switchToBrowserTab(orcaPage, worktreeId, sourceTab!.id) await clickBrowserLink(orcaPage, sourceTab!.id, '#frame-modifier-link', { frameSelector: '#link-frame',