From ac116248f963c683eea8b9c5a87ed8579f2ade58 Mon Sep 17 00:00:00 2001 From: Julian Descottes Date: Fri, 10 Jan 2025 19:14:11 +0100 Subject: [PATCH 1/2] chore(bidi): Avoid errors if url is unavailable in navigationStarted event --- packages/playwright-core/src/server/bidi/bidiPage.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/playwright-core/src/server/bidi/bidiPage.ts b/packages/playwright-core/src/server/bidi/bidiPage.ts index cf0662738b1d4..e27bbfca7dc5d 100644 --- a/packages/playwright-core/src/server/bidi/bidiPage.ts +++ b/packages/playwright-core/src/server/bidi/bidiPage.ts @@ -180,6 +180,12 @@ export class BidiPage implements PageDelegate { const frameId = params.context; this._page._frameManager.frameRequestedNavigation(frameId, params.navigation!); + // url is missing from navigationStarted events on Firefox when the + // navigation is interrupted by a beforeunload prompt. + // See https://bugzilla.mozilla.org/show_bug.cgi?id=1908952 + if (!params.url) + return; + const url = params.url.toLowerCase(); if (url.startsWith('file:') || url.startsWith('data:') || url === 'about:blank') { // Navigation to file urls doesn't emit network events, so we fire 'commit' event right when navigation is started. From 8778e022178f682579fcb437bab928765cdfd788 Mon Sep 17 00:00:00 2001 From: Julian Descottes Date: Fri, 10 Jan 2025 19:15:10 +0100 Subject: [PATCH 2/2] chore(bidi): Do not wait for browsingContext.close if runBeforeUnload is true --- packages/playwright-core/src/server/bidi/bidiPage.ts | 9 ++++++++- tests/bidi/expectations/bidi-chromium-library.txt | 2 -- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/playwright-core/src/server/bidi/bidiPage.ts b/packages/playwright-core/src/server/bidi/bidiPage.ts index e27bbfca7dc5d..757a269c6d701 100644 --- a/packages/playwright-core/src/server/bidi/bidiPage.ts +++ b/packages/playwright-core/src/server/bidi/bidiPage.ts @@ -390,10 +390,17 @@ export class BidiPage implements PageDelegate { } async closePage(runBeforeUnload: boolean): Promise { - await this._session.send('browsingContext.close', { + const onClose = this._session.send('browsingContext.close', { context: this._session.sessionId, promptUnload: runBeforeUnload, }); + + // Only wait for the browsingContext to close if runBeforeUnload is false. + // Otherwise a before unload prompt might be displayed and should be handled + // by the caller. + // See NOTE on https://playwright.dev/docs/api/class-page#page-close + if (!runBeforeUnload) + await onClose; } async setBackgroundColor(color?: { r: number; g: number; b: number; a: number; }): Promise { diff --git a/tests/bidi/expectations/bidi-chromium-library.txt b/tests/bidi/expectations/bidi-chromium-library.txt index aad83691f69b1..091e3a58e4afa 100644 --- a/tests/bidi/expectations/bidi-chromium-library.txt +++ b/tests/bidi/expectations/bidi-chromium-library.txt @@ -1,5 +1,3 @@ -library/beforeunload.spec.ts › should access page after beforeunload [timeout] -library/beforeunload.spec.ts › should run beforeunload if asked for @smoke [timeout] library/browsercontext-events.spec.ts › console event should work in immediately closed popup [timeout] library/browsercontext-events.spec.ts › console event should work in popup 2 [timeout] library/browsercontext-events.spec.ts › dialog event should work in immediately closed popup [timeout]