From bd4ffd42126d689c71929dec50fc0b0c9e1c5e62 Mon Sep 17 00:00:00 2001 From: "Abnoz.v2" Date: Sun, 20 Sep 2026 13:59:58 +0100 Subject: [PATCH] fix(mcp): report a dialog opened during navigation instead of timing out A dialog opened while the page loads blocks domcontentloaded, so browser_navigate waited for the whole navigation timeout before failing. Race the navigation against a new modal state and return as soon as the dialog shows up, the same way actions do. Fixes https://github.com/microsoft/playwright/issues/42817 --- .../playwright-core/src/tools/backend/tab.ts | 14 +++++++++- tests/mcp/dialogs.spec.ts | 26 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/playwright-core/src/tools/backend/tab.ts b/packages/playwright-core/src/tools/backend/tab.ts index ab0a29d64b2f1..80dacc5daa513 100644 --- a/packages/playwright-core/src/tools/backend/tab.ts +++ b/packages/playwright-core/src/tools/backend/tab.ts @@ -348,8 +348,16 @@ export class Tab extends EventEmitter { this._clearCollectedArtifacts(); const { promise: downloadEvent, abort: abortDownloadEvent } = eventWaiter(this.page, 'download', 3000); + // A dialog that opens during the load blocks it, so report the dialog + // right away instead of waiting for the navigation timeout. + const modalStatePromise = new ManualPromise(); + const modalStateListener = () => modalStatePromise.resolve(); + this.once(TabEvents.modalState, modalStateListener); try { - await this.page.goto(url, { waitUntil: 'domcontentloaded', ...this.navigationTimeoutOptions }); + await Promise.race([ + this.page.goto(url, { waitUntil: 'domcontentloaded', ...this.navigationTimeoutOptions }), + modalStatePromise, + ]); abortDownloadEvent(); } catch (_e: unknown) { const e = _e as Error; @@ -363,7 +371,11 @@ export class Tab extends EventEmitter { // Make sure other "download" listeners are notified first. await new Promise(resolve => setTimeout(resolve, 500)); return; + } finally { + this.off(TabEvents.modalState, modalStateListener); } + if (modalStatePromise.isDone()) + return; // Cap load event to 5 seconds, the page is operational at this point. await this.waitForLoadState('load', { timeout: 5000 }); diff --git a/tests/mcp/dialogs.spec.ts b/tests/mcp/dialogs.spec.ts index 6b13ba0803fbe..ee7c80da7a0be 100644 --- a/tests/mcp/dialogs.spec.ts +++ b/tests/mcp/dialogs.spec.ts @@ -293,3 +293,29 @@ test('alert dialog w/ race', async ({ client, server }) => { - Page Title: Title`), }); }); + +test('alert dialog during navigation', async ({ client, server }) => { + server.setContent('/', `Title`, 'text/html'); + expect(await client.callTool({ + name: 'browser_navigate', + arguments: { url: server.PREFIX }, + })).toHaveResponse({ + modalState: expect.stringContaining(`- ["alert" dialog with message "Alert"]: can be handled by browser_handle_dialog`), + }); + + expect(await client.callTool({ + name: 'browser_handle_dialog', + arguments: { accept: true }, + })).toHaveResponse({ + modalState: undefined, + page: expect.stringContaining(`- Page URL: ${server.PREFIX}/ +- Page Title: Title`), + }); + + expect(await client.callTool({ + name: 'browser_snapshot', + arguments: {}, + })).toHaveResponse({ + inlineSnapshot: expect.stringContaining(`- button "Button"`), + }); +});