"browsingContext.close" should be able to close the last tab of a window
Categories
(Remote Protocol :: WebDriver BiDi, defect, P2)
Tracking
(firefox-esr115 unaffected, firefox121 unaffected, firefox122 unaffected, firefox123 wontfix, firefox124 fixed)
Tracking | Status | |
---|---|---|
firefox-esr115 | --- | unaffected |
firefox121 | --- | unaffected |
firefox122 | --- | unaffected |
firefox123 | --- | wontfix |
firefox124 | --- | fixed |
People
(Reporter: intermittent-bug-filer, Assigned: whimboo)
References
(Regression)
Details
(Keywords: intermittent-failure, intermittent-testcase, regression, Whiteboard: [webdriver:m10][webdriver:relnote])
Attachments
(2 files)
Filed by: hskupin [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer?job_id=442845038&repo=try
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/fhR4hJ3GTKuQ5si51KOX9A/runs/0/artifacts/public/logs/live_backing.log
The test will start to fail because of the upcoming changes from bug 1873916 and especially the preference `browser.tabs.closeWindowWithLastTab` that we set to `false` via recommended preferences for all Remote Agent related protocols.
We should figure out what to do here given that it should only affect the last tab of the very last open browser window. Maybe instead of setting the preference we should use the window enumerator and check if closing the window is possible or not so that the browser doesn't shut down.
Comment 1•8 months ago
|
||
:whimboo, since you are the author of the regressor, bug 1873916, could you take a look?
For more information, please visit BugBot documentation.
Assignee | ||
Comment 2•8 months ago
|
||
This needs a discussion. We will do that in our next weeks triage session.
Comment 3•8 months ago
|
||
Set release status flags based on info from the regressing bug 1873916
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 5•8 months ago
•
|
||
The WebDriver BiDi specification is not clear about the right behavior: https://github.com/w3c/webdriver-bidi/issues/187.
But that shouldn't block us here. So just let us remove this pref and check for at least one open tab/window.
Assignee | ||
Comment 6•8 months ago
|
||
Actually due to windowless mode on MacOS for Marionette we already have checks present:
- https://searchfox.org/mozilla-central/rev/9c65def36af441133c75a44b126e65184b039b2f/remote/marionette/driver.sys.mjs#2306-2311
- https://searchfox.org/mozilla-central/rev/9c65def36af441133c75a44b126e65184b039b2f/remote/marionette/driver.sys.mjs#2348-2350
That means that we only need a fix for WebDriver BiDi.
Assignee | ||
Comment 7•8 months ago
|
||
This also removes the custom value of the recommended
preference browser.tabs.closeWindowWithLastTab
because
it currently blocks closing the last tab of a window for
the "browsingContext.close" command.
Updated•8 months ago
|
Assignee | ||
Updated•8 months ago
|
Updated•8 months ago
|
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/44ed20f894fb [webdriver-bidi] "browsingContext.close" should be able to close the last tab of a window. r=webdriver-reviewers,Sasha
Comment 9•8 months ago
•
|
||
Backed out for causing Puppeteer related failure
Comment 10•8 months ago
|
||
Set release status flags based on info from the regressing bug 1873916
Assignee | ||
Comment 11•8 months ago
|
||
I cannot actually reproduce the problem locally on MacOS. It might be related to the fact that the application stays open even with the last window closed. I actually would propose to keep the browser.tabs.closeWindowWithLastTab
preference set to false
for Puppeteer and only when the CDP protocol is selected. It works all fine with WebDriver BiDi.
I pushed a try build to check if that approach works:
https://treeherder.mozilla.org/jobs?repo=try&revision=2a46d284a5b154792a517a8f431a416e4206fb97
Assignee | ||
Comment 12•8 months ago
|
||
Assignee | ||
Comment 13•8 months ago
|
||
I was able to reproduce on a Linux machine but fixing this might need some effort that we do not want to spend for CDP. As such keeping the preference for CDP sounds the best solution to me.
The only problem is that we cannot land the patch stack until a new version of Puppeteer has been released with the change in D200073 included.
Assignee | ||
Updated•8 months ago
|
Assignee | ||
Comment 14•8 months ago
|
||
The upstream PR for Puppeteer is: https://github.com/puppeteer/puppeteer/pull/11794
Assignee | ||
Comment 15•8 months ago
|
||
(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #14)
The upstream PR for Puppeteer is: https://github.com/puppeteer/puppeteer/pull/11794
The PR got merged. I'm now waiting for the next Puppeteer release to get this stack of patches pushed.
Assignee | ||
Comment 16•8 months ago
|
||
A new Puppeteer release is out. That means we can push the stack now.
Comment 17•8 months ago
|
||
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/539c2d1bb3b6 [webdriver-bidi] "browsingContext.close" should be able to close the last tab of a window. r=webdriver-reviewers,Sasha https://hg.mozilla.org/integration/autoland/rev/d4f5993f9b5c [puppeteer] Keep "browser.tabs.closeWindowWithLastTab" disabled for CDP. r=webdriver-reviewers,Sasha
Comment 18•8 months ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/539c2d1bb3b6
https://hg.mozilla.org/mozilla-central/rev/d4f5993f9b5c
Comment 19•8 months ago
|
||
The patch landed in nightly and beta is affected.
:whimboo, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval.
- If no, please set
status-firefox123
towontfix
.
For more information, please visit BugBot documentation.
Assignee | ||
Comment 20•8 months ago
|
||
This should as best ride the trains, so lets not uplift.
Assignee | ||
Updated•6 months ago
|
Description
•