From ee13248d1d9eeb4f04db07f26f143a5591172336 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sat, 9 May 2026 20:04:07 -0700 Subject: [PATCH 1/2] fix(bidi): guard nullable navigation fields in event handlers --- packages/playwright-core/src/server/bidi/bidiPage.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/playwright-core/src/server/bidi/bidiPage.ts b/packages/playwright-core/src/server/bidi/bidiPage.ts index 6637b0d46ee96..153303569839e 100644 --- a/packages/playwright-core/src/server/bidi/bidiPage.ts +++ b/packages/playwright-core/src/server/bidi/bidiPage.ts @@ -194,14 +194,17 @@ export class BidiPage implements PageDelegate { private _onNavigationStarted(params: bidi.BrowsingContext.NavigationInfo) { const frameId = params.context; - this._page.frameManager.frameRequestedNavigation(frameId, params.navigation!); + this._page.frameManager.frameRequestedNavigation(frameId, params.navigation || undefined); } private _onNavigationCommitted(params: bidi.BrowsingContext.NavigationInfo) { const frameId = params.context; - const frame = this._page.frameManager.frame(frameId)!; + const frame = this._page.frameManager.frame(frameId); + if (!frame) + return; this._browserContext.doGrantGlobalPermissionsForURL(params.url).catch(error => debugLogger.log('error', error)); - this._page.frameManager.frameCommittedNewDocumentNavigation(frameId, params.url, frame._name, params.navigation!, /* initial */ false); + if (params.navigation) + this._page.frameManager.frameCommittedNewDocumentNavigation(frameId, params.url, frame._name, params.navigation, /* initial */ false); } private _onDomContentLoaded(params: bidi.BrowsingContext.NavigationInfo) { From f97b6aea8bf6da3fc82fc286da84bb7723119adc Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sun, 10 May 2026 17:41:19 -0700 Subject: [PATCH 2/2] test(bidi): add test for null navigation handling Add test to verify BiDi navigation events with null navigation are handled gracefully. Tests navigation to about:blank, history.pushState, and goBack scenarios. --- tests/page/page-goto.spec.ts | 38 ++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/page/page-goto.spec.ts b/tests/page/page-goto.spec.ts index d6bbad9856e37..e5efc4abac4c6 100644 --- a/tests/page/page-goto.spec.ts +++ b/tests/page/page-goto.spec.ts @@ -254,6 +254,44 @@ it('should navigate to about:blank', async ({ page, server }) => { expect(response).toBe(null); }); +it('should handle BiDi navigationCommitted with null navigation id', async ({ page, server, toImpl, isBidi }) => { + it.skip(!isBidi, 'BiDi-specific test'); + + await page.goto(server.EMPTY_PAGE); + + // Access internal BiDi page to simulate a spec-compliant event. + // W3C BiDi spec defines BaseNavigationInfo.navigation as + // "Navigation | null" — null when navigation is canceled before + // making progress (w3c/webdriver-bidi#766). + const pageImpl = toImpl(page); + const mainFrame = pageImpl.mainFrame(); + const bidiPage = pageImpl.delegate; + const browserContext = bidiPage._browserContext; + + // Stub doGrantGlobalPermissionsForURL to prevent async BiDi commands + // that would race with test teardown. + const origGrant = browserContext.doGrantGlobalPermissionsForURL.bind(browserContext); + browserContext.doGrantGlobalPermissionsForURL = async () => {}; + + try { + // Call handler with navigation: null. + // Without the null guard, frameCommittedNewDocumentNavigation() runs, + // overwriting frame._url and clearing lifecycle events. + bidiPage._onNavigationCommitted({ + context: mainFrame._id, + navigation: null, + timestamp: Date.now(), + url: server.PREFIX + '/injected', + }); + + // Without the fix: frame._url was overwritten to server.PREFIX + '/injected' + // With the fix: the if (params.navigation) guard skips the call entirely. + expect(mainFrame._url).toBe(server.EMPTY_PAGE); + } finally { + browserContext.doGrantGlobalPermissionsForURL = origGrant; + } +}); + it('should return response when page changes its URL after load', async ({ page, server }) => { const response = await page.goto(server.PREFIX + '/historyapi.html'); expect(response.status()).toBe(200);