Browser.close should not trigger onbeforeunload dialogs
Categories
(Remote Protocol :: CDP, defect, P3)
Tracking
(Not tracked)
People
(Reporter: jdescottes, Unassigned)
References
Details
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.
Reporter | ||
Comment 1•5 years ago
|
||
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!
Reporter | ||
Comment 2•5 years ago
|
||
Feel free to pick this up!
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 3•5 years ago
|
||
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.
Updated•5 years ago
|
Updated•5 years ago
|
Ugh, sorry for the noise. This isn't blocking gutenberg tests in the way that I thought, after all.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Updated•4 years ago
|
Updated•2 years ago
|
Description
•