fix(dialogs): let click timeout, log information about dialogs

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) => {

src/dialog.ts

@@ -16,23 +16,27 @@
  */
 
 import { assert } from './helper';
+import { Logger } from './logger';
 
 type OnHandle = (accept: boolean, promptText?: string) => Promise<void>;
 
 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'}"`;
+  }
 }

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) => {

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<Element>;
-        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') {

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}) {

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(`<div onclick='window.alert(123)'>Click me</div>`);
+    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() {

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();
+  });
 });