1567476 - Browser.close should not trigger onbeforeunload dialogs

Status: RESOLVED WONTFIX

(Remote Protocol :: CDP, defect, P3)

Description

[Julian Descottes [:jdescottes]]

Pending confirmation from Chromium peers, the documentation is not really clear on this topic, but juggler implemented Browser.close in a way that bypasses onbeforeunload:

  async close() {
    Services.startup.quit(Ci.nsIAppStartup.eForceQuit);
  }

https://github.com/puppeteer/juggler/blob/master/testing/juggler/protocol/BrowserHandler.js#L17

Instead we used

  close() {
    Services.startup.quit(Ci.nsIAppStartup.eAttemptQuit);
  }

which will trigger onbeforeunload popups.

This seems to be confirmed when running the same gutenberg test involving onbeforeunload prompts side by side between chrome and firefox. The chrome run will trigger 1 less beforeunload event compared to firefox.

Concerning the impact on Gutenberg, this makes any test that triggers onbeforeunload prompts timeout at the end of the suite. This is much more inconvenient when running tests one by one, because it will make each test fail.

Comment 1

[Julian Descottes [:jdescottes]]

Pending confirmation from Chromium peers,
Quoting Andrey on slack: "browser.close() is expected to close browser unconditionally"

I'll move forward with the change here, although I'm not sure how to write a test for this at the moment.
Any suggestion appreciated!

Comment 2

[Julian Descottes [:jdescottes]]

Feel free to pick this up!

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

General issue with test pages which have a beforeunload event handler registered. Given that the Gutenberg editor has such a handler setup, we should make sure to be able to close the browser window. Otherwise the dialog remains open, and would block further interaction with the window. As such keeping it on the radar for the alpha release.

Comment 5

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Ugh, sorry for the noise. This isn't blocking gutenberg tests in the way that I thought, after all.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

We are not going to implement this feature for CDP but we will have it eventually for WebDriver BiDi. See bug 1824220.