refactor: use helper for Dialog handle (#2334)

Re-use the logic in the wait for helper to expose per dialog type
handling.
This commit is contained in:
Nikolay Vitkov
2026-07-09 17:12:48 +02:00
committed by GitHub
parent 9cd734b0a3
commit 64005f924f
8 changed files with 70 additions and 45 deletions
+1 -1
View File
@@ -208,7 +208,7 @@
**Parameters:**
- **handleBeforeUnload** (enum: "accept", "decline") _(optional)_: Whether to auto accept or beforeunload dialogs triggered by this navigation. Default is accept.
- **handleBeforeUnload** (enum: "accept", "dismiss") _(optional)_: Whether to auto accept or beforeunload dialogs triggered by this navigation. Default is accept.
- **ignoreCache** (boolean) _(optional)_: Whether to ignore cache on reload.
- **initScript** (string) _(optional)_: A JavaScript script to be executed on each new document before any other scripts for the next navigation.
- **timeout** (integer) _(optional)_: Maximum wait time in milliseconds. If set to 0, the default timeout will be used.
+9 -7
View File
@@ -14,9 +14,10 @@ import {TextSnapshot} from './TextSnapshot.js';
import type {
Dialog,
ElementHandle,
Page,
Viewport,
WebMCPTool,
Protocol,
Page,
} from './third_party/index.js';
import {takeSnapshot} from './tools/snapshot.js';
import type {ToolGroups} from './tools/thirdPartyDeveloper.js';
@@ -34,6 +35,7 @@ import {
getNetworkMultiplierFromString,
WaitForHelper,
type WaitForEventsResult,
type DialogAction,
} from './WaitForHelper.js';
/**
@@ -93,12 +95,8 @@ export class McpPage implements ContextPage {
});
}
get dialog(): Dialog | undefined {
return this.#dialog;
}
getDialog(): Dialog | undefined {
return this.dialog;
return this.#dialog;
}
clearDialog(): void {
@@ -155,7 +153,11 @@ export class McpPage implements ContextPage {
waitForEventsAfterAction(
action: () => Promise<unknown>,
options?: {timeout?: number; handleDialog?: 'accept' | 'dismiss' | string},
options?: {
timeout?: number;
handleDialog?:
DialogAction | Partial<Record<Protocol.Page.DialogType, DialogAction>>;
},
): Promise<WaitForEventsResult> {
const helper = this.createWaitForHelper(
this.cpuThrottlingRate,
+31 -8
View File
@@ -8,6 +8,8 @@ import {logger} from './logger.js';
import type {Page, Protocol, CdpPage, Dialog} from './third_party/index.js';
import type {PredefinedNetworkConditions} from './third_party/index.js';
export type DialogAction = 'accept' | 'dismiss' | string;
export class WaitForHelper {
#abortController = new AbortController();
#page: CdpPage;
@@ -130,20 +132,36 @@ export class WaitForHelper {
async waitForEventsAfterAction(
action: () => Promise<unknown>,
options?: {timeout?: number; handleDialog?: 'accept' | 'dismiss' | string},
options?: {
timeout?: number;
handleDialog?:
DialogAction | Partial<Record<Protocol.Page.DialogType, DialogAction>>;
},
): Promise<WaitForEventsResult> {
if (this.#abortController.signal.aborted) {
throw new Error("Can't re-use a WaitForHelper");
}
if (options?.handleDialog) {
const dialogHandler = (dialog: Pick<Dialog, 'accept' | 'dismiss'>) => {
this.#dialogOpened = true;
if (options.handleDialog === 'dismiss') {
void dialog.dismiss();
} else if (options.handleDialog === 'accept') {
void dialog.accept();
const dialogHandler = (
dialog: Pick<Dialog, 'accept' | 'dismiss' | 'type'>,
) => {
let actionToTake: DialogAction | undefined;
if (typeof options.handleDialog === 'object') {
actionToTake = options.handleDialog[dialog.type()];
} else {
void dialog.accept(options.handleDialog);
actionToTake = options.handleDialog;
}
if (actionToTake) {
this.#dialogOpened = true;
if (actionToTake === 'dismiss') {
void dialog.dismiss();
} else if (actionToTake === 'accept') {
void dialog.accept();
} else {
void dialog.accept(actionToTake);
}
}
};
this.#page.on('dialog', dialogHandler);
@@ -197,6 +215,7 @@ export class WaitForHelper {
...(urlAfterAction !== this.#initialUrl
? {navigatedToUrl: urlAfterAction}
: {}),
dialogHandled: this.#dialogOpened,
};
}
}
@@ -207,6 +226,10 @@ export interface WaitForEventsResult {
* occurred.
*/
navigatedToUrl?: string;
/**
* Whether a dialog was automatically handled during the action.
*/
dialogHandled?: boolean;
}
export function getNetworkMultiplierFromString(
+1 -1
View File
@@ -812,7 +812,7 @@ export const commands: Commands = {
description:
'Whether to auto accept or beforeunload dialogs triggered by this navigation. Default is accept.',
required: false,
enum: ['accept', 'decline'],
enum: ['accept', 'dismiss'],
},
initScript: {
name: 'initScript',
+8 -3
View File
@@ -17,10 +17,11 @@ import type {
Dialog,
ElementHandle,
Extension,
Page,
ScreenRecorder,
Viewport,
DevTools,
Protocol,
Page,
} from '../third_party/index.js';
import type {InsightName, TraceResult} from '../trace-processing/parse.js';
import type {
@@ -29,7 +30,7 @@ import type {
ExtensionServiceWorker,
} from '../types.js';
import type {PaginationOptions} from '../utils/types.js';
import type {WaitForEventsResult} from '../WaitForHelper.js';
import type {WaitForEventsResult, DialogAction} from '../WaitForHelper.js';
import type {ToolCategory} from './categories.js';
import type {ToolGroups} from './thirdPartyDeveloper.js';
@@ -316,7 +317,11 @@ export type ContextPage = Readonly<{
throwIfDialogOpen(): void;
waitForEventsAfterAction(
action: () => Promise<unknown>,
options?: {timeout?: number; handleDialog?: 'accept' | 'dismiss' | string},
options?: {
timeout?: number;
handleDialog?:
DialogAction | Partial<Record<Protocol.Page.DialogType, DialogAction>>;
},
): Promise<WaitForEventsResult>;
getThirdPartyDeveloperTools(): ToolGroups;
+14 -22
View File
@@ -5,7 +5,7 @@
*/
import {logger} from '../logger.js';
import type {CdpPage, Dialog} from '../third_party/index.js';
import type {CdpPage} from '../third_party/index.js';
import {zod} from '../third_party/index.js';
import {ToolCategory} from './categories.js';
@@ -166,7 +166,7 @@ export const navigatePage = definePageTool(() => {
.optional()
.describe('Whether to ignore cache on reload.'),
handleBeforeUnload: zod
.enum(['accept', 'decline'])
.enum(['accept', 'dismiss'])
.optional()
.describe(
'Whether to auto accept or beforeunload dialogs triggered by this navigation. Default is accept.',
@@ -195,21 +195,6 @@ export const navigatePage = definePageTool(() => {
request.params.type = 'url';
}
const handleBeforeUnload = request.params.handleBeforeUnload ?? 'accept';
const dialogHandler = (dialog: Dialog) => {
if (dialog.type() === 'beforeunload') {
if (handleBeforeUnload === 'accept') {
response.appendResponseLine(`Accepted a beforeunload dialog.`);
void dialog.accept();
} else {
response.appendResponseLine(`Declined a beforeunload dialog.`);
void dialog.dismiss();
}
// We are not going to report the dialog like regular dialogs.
page.clearDialog();
}
};
let initScriptId: string | undefined;
if (request.params.initScript) {
const {identifier} = await page.pptrPage.evaluateOnNewDocument(
@@ -218,10 +203,9 @@ export const navigatePage = definePageTool(() => {
initScriptId = identifier;
}
page.pptrPage.on('dialog', dialogHandler);
try {
await page.waitForEventsAfterAction(
const action = request.params.handleBeforeUnload ?? 'accept';
const result = await page.waitForEventsAfterAction(
async () => {
switch (request.params.type) {
case 'url':
@@ -282,10 +266,18 @@ export const navigatePage = definePageTool(() => {
break;
}
},
{timeout: request.params.timeout},
{
timeout: request.params.timeout,
handleDialog: {beforeunload: action},
},
);
if (result.dialogHandled) {
response.appendResponseLine(
`${action === 'dismiss' ? 'Dismissed' : 'Accepted'} a beforeunload dialog.`,
);
page.clearDialog();
}
} finally {
page.pptrPage.off('dialog', dialogHandler);
if (initScriptId) {
await page.pptrPage
.removeScriptToEvaluateOnNewDocument(initScriptId)
+3
View File
@@ -97,6 +97,9 @@ Example with arguments: \`(el) => el.innerText\`
},
{handleDialog: dialogAction ?? 'accept'},
);
if (result.dialogHandled) {
context.getSelectedMcpPage().clearDialog();
}
response.attachWaitForResult(result);
return;
}
+3 -3
View File
@@ -773,7 +773,7 @@ describe('pages', () => {
assert.ok(response.includePages);
assert.strictEqual(
response.responseLines.join('\n'),
'Accepted a beforeunload dialog.\nSuccessfully reloaded the page.',
'Successfully reloaded the page.\nAccepted a beforeunload dialog.',
);
});
});
@@ -794,7 +794,7 @@ describe('pages', () => {
{
params: {
type: 'reload',
handleBeforeUnload: 'decline',
handleBeforeUnload: 'dismiss',
timeout: 500,
},
page: context.getSelectedMcpPage(),
@@ -807,7 +807,7 @@ describe('pages', () => {
assert.ok(response.includePages);
assert.strictEqual(
response.responseLines.join('\n'),
'Declined a beforeunload dialog.\nUnable to reload the selected page: Navigation timeout of 500 ms exceeded.',
'Unable to reload the selected page: Navigation timeout of 500 ms exceeded.\nDismissed a beforeunload dialog.',
);
});
});