diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts index 66b9a485ff..95ab316643 100644 --- a/src/chromium/crPage.ts +++ b/src/chromium/crPage.ts @@ -650,6 +650,7 @@ class FrameSession { _onDialog(event: Protocol.Page.javascriptDialogOpeningPayload) { this._page.emit(Events.Page.Dialog, new dialog.Dialog( + this._page._logger, event.type, event.message, async (accept: boolean, promptText?: string) => { diff --git a/src/dialog.ts b/src/dialog.ts index c439b987bd..5f7db6adef 100644 --- a/src/dialog.ts +++ b/src/dialog.ts @@ -16,23 +16,27 @@ */ import { assert } from './helper'; +import { Logger } from './logger'; type OnHandle = (accept: boolean, promptText?: string) => Promise; export type DialogType = 'alert' | 'beforeunload' | 'confirm' | 'prompt'; export class Dialog { + private _logger: Logger; private _type: string; private _message: string; private _onHandle: OnHandle; private _handled = false; private _defaultValue: string; - constructor(type: string, message: string, onHandle: OnHandle, defaultValue?: string) { + constructor(logger: Logger, type: string, message: string, onHandle: OnHandle, defaultValue?: string) { + this._logger = logger; this._type = type; this._message = message; this._onHandle = onHandle; this._defaultValue = defaultValue || ''; + this._logger.info(` ${this._preview()} was shown`); } type(): string { @@ -50,12 +54,22 @@ export class Dialog { async accept(promptText: string | undefined) { assert(!this._handled, 'Cannot accept dialog which is already handled!'); this._handled = true; + this._logger.info(` ${this._preview()} was accepted`); await this._onHandle(true, promptText); } async dismiss() { assert(!this._handled, 'Cannot dismiss dialog which is already handled!'); this._handled = true; + this._logger.info(` ${this._preview()} was dismissed`); await this._onHandle(false); } + + private _preview(): string { + if (!this._message) + return this._type; + if (this._message.length <= 50) + return `${this._type} "${this._message}"`; + return `${this._type} "${this._message.substring(0, 49) + '\u2026'}"`; + } } diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts index 48c9a40f66..304166ebf2 100644 --- a/src/firefox/ffPage.ts +++ b/src/firefox/ffPage.ts @@ -188,6 +188,7 @@ export class FFPage implements PageDelegate { _onDialogOpened(params: Protocol.Page.dialogOpenedPayload) { this._page.emit(Events.Page.Dialog, new dialog.Dialog( + this._page._logger, params.type, params.message, async (accept: boolean, promptText?: string) => { diff --git a/src/frames.ts b/src/frames.ts index 6096f4e191..f3be8b2af4 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -726,7 +726,11 @@ export class Frame { const task = dom.waitForSelectorTask(info, 'attached'); const handle = await this._scheduleRerunnableHandleTask(progress, info.world, task); const element = handle.asElement() as dom.ElementHandle; - progress.cleanupWhenAborted(() => element.dispose()); + progress.cleanupWhenAborted(() => { + // Do not await here to avoid being blocked, either by stalled + // page (e.g. alert) or unresolved navigation in Chromium. + element.dispose(); + }); const result = await action(progress, element); element.dispose(); if (result === 'error:notconnected') { diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index e4d7438480..6f58c4279d 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -519,12 +519,13 @@ export class WKPage implements PageDelegate { _onDialog(event: Protocol.Dialog.javascriptDialogOpeningPayload) { this._page.emit(Events.Page.Dialog, new dialog.Dialog( - event.type as dialog.DialogType, - event.message, - async (accept: boolean, promptText?: string) => { - await this._pageProxySession.send('Dialog.handleJavaScriptDialog', { accept, promptText }); - }, - event.defaultPrompt)); + this._page._logger, + event.type as dialog.DialogType, + event.message, + async (accept: boolean, promptText?: string) => { + await this._pageProxySession.send('Dialog.handleJavaScriptDialog', { accept, promptText }); + }, + event.defaultPrompt)); } private async _onFileChooserOpened(event: {frameId: Protocol.Network.FrameId, element: Protocol.Runtime.RemoteObject}) { diff --git a/test/click.spec.js b/test/click.spec.js index 313d4dc8a6..248c2ee79e 100644 --- a/test/click.spec.js +++ b/test/click.spec.js @@ -835,6 +835,14 @@ describe('Page.click', function() { await page.click('button'); expect(await page.evaluate(() => result)).toBe('Clicked'); }); + it('should timeout when click opens alert', async({page, server}) => { + const dialogPromise = page.waitForEvent('dialog'); + await page.setContent(`
Click me
`); + const error = await page.click('div', { timeout: 3000 }).catch(e => e); + expect(error.message).toContain('Timeout 3000ms exceeded during page.click.'); + const dialog = await dialogPromise; + await dialog.dismiss(); + }); }); describe('Page.check', function() { diff --git a/test/dialog.spec.js b/test/dialog.spec.js index 89b71a4b79..dc50793a62 100644 --- a/test/dialog.spec.js +++ b/test/dialog.spec.js @@ -15,7 +15,7 @@ * limitations under the License. */ -const {FFOX, CHROMIUM, WEBKIT} = require('./utils').testOptions(browserType); +const {FFOX, CHROMIUM, WEBKIT, CHANNEL} = require('./utils').testOptions(browserType); describe('Page.Events.Dialog', function() { it('should fire', async({page, server}) => { @@ -58,4 +58,31 @@ describe('Page.Events.Dialog', function() { const result = await page.evaluate(() => confirm('boolean?')); expect(result).toBe(false); }); + it.fail(CHANNEL)('should log prompt actions', async({browser}) => { + const messages = []; + const context = await browser.newContext({ + logger: { + isEnabled: () => true, + log: (name, severity, message) => messages.push(message), + } + }); + const page = await context.newPage(); + const promise = page.evaluate(() => confirm('01234567890123456789012345678901234567890123456789012345678901234567890123456789')); + const dialog = await page.waitForEvent('dialog'); + expect(messages.join()).toContain('confirm "0123456789012345678901234567890123456789012345678…" was shown'); + await dialog.accept('123'); + await promise; + expect(messages.join()).toContain('confirm "0123456789012345678901234567890123456789012345678…" was accepted'); + await context.close(); + }); + it.fail(WEBKIT)('should be able to close context with open alert', async({browser}) => { + const context = await browser.newContext(); + const page = await context.newPage(); + const alertPromise = page.waitForEvent('dialog'); + await page.evaluate(() => { + setTimeout(() => alert('hello'), 0); + }); + await alertPromise; + await context.close(); + }); });