Skip to content

Commit

Permalink
feat: BrowserContext.on('dialog') (#22033)
Browse files Browse the repository at this point in the history
Dialogs created early during page initialization are only reported on
the context, with `page()` being `null`.
  • Loading branch information
dgozman committed Mar 28, 2023
1 parent 00d9877 commit 3b359e2
Show file tree
Hide file tree
Showing 23 changed files with 342 additions and 62 deletions.
36 changes: 36 additions & 0 deletions docs/src/api/class-browsercontext.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,42 @@ context.Console += async (_, msg) =>
await page.EvaluateAsync("console.log('hello', 5, { foo: 'bar' })");
```


## event: BrowserContext.dialog
* since: v1.33
- argument: <[Dialog]>

Emitted when a JavaScript dialog appears, such as `alert`, `prompt`, `confirm` or `beforeunload`. Listener **must** either [`method: Dialog.accept`] or [`method: Dialog.dismiss`] the dialog - otherwise the page will [freeze](https://developer.mozilla.org/en-US/docs/Web/JavaScript/EventLoop#never_blocking) waiting for the dialog, and actions like click will never finish.

**Usage**

```js
context.on('dialog', dialog => {
dialog.accept();
});
```

```java
context.onDialog(dialog -> {
dialog.accept();
});
```

```python
context.on("dialog", lambda dialog: dialog.accept())
```

```csharp
context.RequestFailed += (_, request) =>
{
Console.WriteLine(request.Url + " " + request.Failure);
};
```

:::note
When no [`event: Page.dialog`] or [`event: BrowserContext.dialog`] listeners are present, all dialogs are automatically dismissed.
:::

## event: BrowserContext.page
* since: v1.8
- argument: <[Page]>
Expand Down
6 changes: 6 additions & 0 deletions docs/src/api/class-dialog.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@ Returns when the dialog has been dismissed.

A message displayed in the dialog.

## method: Dialog.page
* since: v1.33
- returns: <[Page]|[null]>

The page that initiated this dialog, if available.

## method: Dialog.type
* since: v1.8
- returns: <[string]>
Expand Down
4 changes: 3 additions & 1 deletion docs/src/api/class-page.md
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,8 @@ try {

Emitted when a JavaScript dialog appears, such as `alert`, `prompt`, `confirm` or `beforeunload`. Listener **must** either [`method: Dialog.accept`] or [`method: Dialog.dismiss`] the dialog - otherwise the page will [freeze](https://developer.mozilla.org/en-US/docs/Web/JavaScript/EventLoop#never_blocking) waiting for the dialog, and actions like click will never finish.

**Usage**

```js
page.on('dialog', dialog => {
dialog.accept();
Expand All @@ -309,7 +311,7 @@ page.RequestFailed += (_, request) =>
```

:::note
When no [`event: Page.dialog`] listeners are present, all dialogs are automatically dismissed.
When no [`event: Page.dialog`] or [`event: BrowserContext.dialog`] listeners are present, all dialogs are automatically dismissed.
:::

## event: Page.DOMContentLoaded
Expand Down
14 changes: 14 additions & 0 deletions packages/playwright-core/src/client/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { createInstrumentation } from './clientInstrumentation';
import { rewriteErrorMessage } from '../utils/stackTrace';
import { HarRouter } from './harRouter';
import { ConsoleMessage } from './consoleMessage';
import { Dialog } from './dialog';

export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel> implements api.BrowserContext {
_pages = new Set<Page>();
Expand Down Expand Up @@ -100,6 +101,19 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
if (page)
page.emit(Events.Page.Console, consoleMessage);
});
this._channel.on('dialog', ({ dialog }) => {
const dialogObject = Dialog.from(dialog);
let hasListeners = this.emit(Events.BrowserContext.Dialog, dialogObject);
const page = dialogObject.page();
if (page)
hasListeners = page.emit(Events.Page.Dialog, dialogObject) || hasListeners;
if (!hasListeners) {
if (dialogObject.type() === 'beforeunload')
dialog.accept({}).catch(() => {});
else
dialog.dismiss().catch(() => {});
}
});
this._channel.on('request', ({ request, page }) => this._onRequest(network.Request.from(request), Page.fromNullable(page)));
this._channel.on('requestFailed', ({ request, failureText, responseEndTiming, page }) => this._onRequestFailed(network.Request.from(request), responseEndTiming, failureText, Page.fromNullable(page)));
this._channel.on('requestFinished', params => this._onRequestFinished(params));
Expand Down
10 changes: 10 additions & 0 deletions packages/playwright-core/src/client/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,24 @@
import type * as channels from '@protocol/channels';
import { ChannelOwner } from './channelOwner';
import type * as api from '../../types/types';
import { Page } from './page';

export class Dialog extends ChannelOwner<channels.DialogChannel> implements api.Dialog {
static from(dialog: channels.DialogChannel): Dialog {
return (dialog as any)._object;
}

private _page: Page | null;

constructor(parent: ChannelOwner, type: string, guid: string, initializer: channels.DialogInitializer) {
super(parent, type, guid, initializer);
// Note: dialogs that open early during page initialization block it.
// Therefore, we must report the dialog without a page to be able to handle it.
this._page = Page.fromNullable(initializer.page);
}

page() {
return this._page;
}

type(): string {
Expand Down
1 change: 1 addition & 0 deletions packages/playwright-core/src/client/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const Events = {
BrowserContext: {
Console: 'console',
Close: 'close',
Dialog: 'dialog',
Page: 'page',
BackgroundPage: 'backgroundpage',
ServiceWorker: 'serviceworker',
Expand Down
10 changes: 0 additions & 10 deletions packages/playwright-core/src/client/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import type { BrowserContext } from './browserContext';
import { ChannelOwner } from './channelOwner';
import { evaluationScript } from './clientHelper';
import { Coverage } from './coverage';
import { Dialog } from './dialog';
import { Download } from './download';
import { determineScreenshotType, ElementHandle } from './elementHandle';
import { Events } from './events';
Expand Down Expand Up @@ -124,15 +123,6 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
this._channel.on('bindingCall', ({ binding }) => this._onBinding(BindingCall.from(binding)));
this._channel.on('close', () => this._onClose());
this._channel.on('crash', () => this._onCrash());
this._channel.on('dialog', ({ dialog }) => {
const dialogObj = Dialog.from(dialog);
if (!this.emit(Events.Page.Dialog, dialogObj)) {
if (dialogObj.type() === 'beforeunload')
dialog.accept({}).catch(() => {});
else
dialog.dismiss().catch(() => {});
}
});
this._channel.on('download', ({ url, suggestedFilename, artifact }) => {
const artifactObject = Artifact.from(artifact);
this.emit(Events.Page.Download, new Download(this, url, suggestedFilename, artifactObject));
Expand Down
7 changes: 4 additions & 3 deletions packages/playwright-core/src/protocol/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,9 @@ scheme.BrowserContextConsoleEvent = tObject({
message: tChannel(['ConsoleMessage']),
});
scheme.BrowserContextCloseEvent = tOptional(tObject({}));
scheme.BrowserContextDialogEvent = tObject({
dialog: tChannel(['Dialog']),
});
scheme.BrowserContextPageEvent = tObject({
page: tChannel(['Page']),
});
Expand Down Expand Up @@ -933,9 +936,6 @@ scheme.PageBindingCallEvent = tObject({
});
scheme.PageCloseEvent = tOptional(tObject({}));
scheme.PageCrashEvent = tOptional(tObject({}));
scheme.PageDialogEvent = tObject({
dialog: tChannel(['Dialog']),
});
scheme.PageDownloadEvent = tObject({
url: tString,
suggestedFilename: tString,
Expand Down Expand Up @@ -2097,6 +2097,7 @@ scheme.BindingCallResolveParams = tObject({
});
scheme.BindingCallResolveResult = tOptional(tObject({}));
scheme.DialogInitializer = tObject({
page: tOptional(tChannel(['Page'])),
type: tString,
message: tString,
defaultValue: tString,
Expand Down
1 change: 1 addition & 0 deletions packages/playwright-core/src/server/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export abstract class BrowserContext extends SdkObject {
static Events = {
Console: 'console',
Close: 'close',
Dialog: 'dialog',
Page: 'page',
Request: 'request',
Response: 'response',
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright-core/src/server/chromium/crPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,7 @@ class FrameSession {
_onDialog(event: Protocol.Page.javascriptDialogOpeningPayload) {
if (!this._page._frameManager.frame(this._targetId))
return; // Our frame/subtree may be gone already.
this._page.emit(Page.Events.Dialog, new dialog.Dialog(
this._page.emitOnContext(BrowserContext.Events.Dialog, new dialog.Dialog(
this._page,
event.type,
event.message,
Expand Down
4 changes: 4 additions & 0 deletions packages/playwright-core/src/server/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ export class Dialog extends SdkObject {
this._page._frameManager.dialogDidOpen(this);
}

page() {
return this._page;
}

type(): string {
return this._type;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import * as path from 'path';
import { createGuid, urlMatches } from '../../utils';
import { WritableStreamDispatcher } from './writableStreamDispatcher';
import { ConsoleMessageDispatcher } from './consoleMessageDispatcher';
import { DialogDispatcher } from './dialogDispatcher';

export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channels.BrowserContextChannel, DispatcherScope> implements channels.BrowserContextChannel {
_type_EventTarget = true;
Expand Down Expand Up @@ -80,7 +81,8 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
this._dispatchEvent('close');
this._dispose();
});
this.addObjectListener(BrowserContext.Events.Console, message => this._dispatchEvent('console', { message: new ConsoleMessageDispatcher(this, message) }));
this.addObjectListener(BrowserContext.Events.Console, message => this._dispatchEvent('console', { message: new ConsoleMessageDispatcher(PageDispatcher.from(this, message.page()), message) }));
this.addObjectListener(BrowserContext.Events.Dialog, dialog => this._dispatchEvent('dialog', { dialog: new DialogDispatcher(this, dialog) }));

if (context._browser.options.name === 'chromium') {
for (const page of (context as CRBrowserContext).backgroundPages())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,15 @@

import type { ConsoleMessage } from '../console';
import type * as channels from '@protocol/channels';
import { PageDispatcher } from './pageDispatcher';
import type { BrowserContextDispatcher } from './browserContextDispatcher';
import type { PageDispatcher } from './pageDispatcher';
import { Dispatcher } from './dispatcher';
import { ElementHandleDispatcher } from './elementHandlerDispatcher';

export class ConsoleMessageDispatcher extends Dispatcher<ConsoleMessage, channels.ConsoleMessageChannel, BrowserContextDispatcher> implements channels.ConsoleMessageChannel {
export class ConsoleMessageDispatcher extends Dispatcher<ConsoleMessage, channels.ConsoleMessageChannel, PageDispatcher> implements channels.ConsoleMessageChannel {
_type_ConsoleMessage = true;

constructor(scope: BrowserContextDispatcher, message: ConsoleMessage) {
const page = PageDispatcher.from(scope, message.page());
super(scope, message, 'ConsoleMessage', {
constructor(page: PageDispatcher, message: ConsoleMessage) {
super(page, message, 'ConsoleMessage', {
type: message.type(),
text: message.text(),
args: message.args().map(a => ElementHandleDispatcher.fromJSHandle(page, a)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,17 @@
import type { Dialog } from '../dialog';
import type * as channels from '@protocol/channels';
import { Dispatcher } from './dispatcher';
import type { PageDispatcher } from './pageDispatcher';
import { PageDispatcher } from './pageDispatcher';
import type { BrowserContextDispatcher } from './browserContextDispatcher';

export class DialogDispatcher extends Dispatcher<Dialog, channels.DialogChannel, PageDispatcher> implements channels.DialogChannel {
export class DialogDispatcher extends Dispatcher<Dialog, channels.DialogChannel, BrowserContextDispatcher | PageDispatcher> implements channels.DialogChannel {
_type_Dialog = true;

constructor(scope: PageDispatcher, dialog: Dialog) {
super(scope, dialog, 'Dialog', {
constructor(scope: BrowserContextDispatcher, dialog: Dialog) {
const page = PageDispatcher.fromNullable(scope, dialog.page().initializedOrUndefined());
// Prefer scoping to the page, unless we don't have one.
super(page || scope, dialog, 'Dialog', {
page,
type: dialog.type(),
message: dialog.message(),
defaultValue: dialog.defaultValue(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import { Page, Worker } from '../page';
import type * as channels from '@protocol/channels';
import { Dispatcher, existingDispatcher } from './dispatcher';
import { parseError, serializeError } from '../../protocol/serializers';
import { DialogDispatcher } from './dialogDispatcher';
import { FrameDispatcher } from './frameDispatcher';
import { RequestDispatcher } from './networkDispatchers';
import { ResponseDispatcher } from './networkDispatchers';
Expand Down Expand Up @@ -76,7 +75,6 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
this._dispose();
});
this.addObjectListener(Page.Events.Crash, () => this._dispatchEvent('crash'));
this.addObjectListener(Page.Events.Dialog, dialog => this._dispatchEvent('dialog', { dialog: new DialogDispatcher(this, dialog) }));
this.addObjectListener(Page.Events.Download, (download: Download) => {
// Artifact can outlive the page, so bind to the context scope.
this._dispatchEvent('download', { url: download.url, suggestedFilename: download.suggestedFilename(), artifact: ArtifactDispatcher.from(parentScope, download.artifact) });
Expand Down
3 changes: 2 additions & 1 deletion packages/playwright-core/src/server/firefox/ffPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import type { Progress } from '../progress';
import { splitErrorMessage } from '../../utils/stackTrace';
import { debugLogger } from '../../common/debugLogger';
import { ManualPromise } from '../../utils/manualPromise';
import { BrowserContext } from '../browserContext';

export const UTILITY_WORLD_NAME = '__playwright_utility_world__';

Expand Down Expand Up @@ -257,7 +258,7 @@ export class FFPage implements PageDelegate {
}

_onDialogOpened(params: Protocol.Page.dialogOpenedPayload) {
this._page.emit(Page.Events.Dialog, new dialog.Dialog(
this._page.emitOnContext(BrowserContext.Events.Dialog, new dialog.Dialog(
this._page,
params.type,
params.message,
Expand Down
31 changes: 18 additions & 13 deletions packages/playwright-core/src/server/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ export class Page extends SdkObject {
static Events = {
Close: 'close',
Crash: 'crash',
Dialog: 'dialog',
Download: 'download',
FileChooser: 'filechooser',
// Can't use just 'error' due to node.js special treatment of error events.
Expand All @@ -140,7 +139,7 @@ export class Page extends SdkObject {
private _closedPromise = new ManualPromise<void>();
private _disconnected = false;
private _initialized = false;
private _consoleMessagesBeforeInitialized: ConsoleMessage[] = [];
private _eventsToEmitAfterInitialized: { event: string | symbol, args: any[] }[] = [];
readonly _disconnectedPromise = new ManualPromise<Error>();
readonly _crashedPromise = new ManualPromise<Error>();
readonly _browserContext: BrowserContext;
Expand Down Expand Up @@ -209,9 +208,9 @@ export class Page extends SdkObject {
this._initialized = true;
this.emitOnContext(contextEvent, this);

for (const message of this._consoleMessagesBeforeInitialized)
this.emitOnContext(BrowserContext.Events.Console, message);
this._consoleMessagesBeforeInitialized = [];
for (const { event, args } of this._eventsToEmitAfterInitialized)
this._browserContext.emit(event, ...args);
this._eventsToEmitAfterInitialized = [];

// It may happen that page initialization finishes after Close event has already been sent,
// in that case we fire another Close event to ensure that each reported Page will have
Expand All @@ -232,6 +231,19 @@ export class Page extends SdkObject {
this._browserContext.emit(event, ...args);
}

emitOnContextOnceInitialized(event: string | symbol, ...args: any[]) {
if (this._isServerSideOnly)
return;
// Some events, like console messages, may come before page is ready.
// In this case, postpone the event until page is initialized,
// and dispatch it to the client later, either on the live Page,
// or on the "errored" Page.
if (this._initialized)
this._browserContext.emit(event, ...args);
else
this._eventsToEmitAfterInitialized.push({ event, args });
}

async resetForReuse(metadata: CallMetadata) {
this.setDefaultNavigationTimeout(undefined);
this.setDefaultTimeout(undefined);
Expand Down Expand Up @@ -361,14 +373,7 @@ export class Page extends SdkObject {
args.forEach(arg => arg.dispose());
return;
}

// Console message may come before page is ready. In this case, postpone the message
// until page is initialized, and dispatch it to the client later, either on the live Page,
// or on the "errored" Page.
if (this._initialized)
this.emitOnContext(BrowserContext.Events.Console, message);
else
this._consoleMessagesBeforeInitialized.push(message);
this.emitOnContextOnceInitialized(BrowserContext.Events.Console, message);
}

async reload(metadata: CallMetadata, options: types.NavigateOptions): Promise<network.Response | null> {
Expand Down

0 comments on commit 3b359e2

Please sign in to comment.