"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•1 year 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•1 year ago
|
||
This needs a discussion. We will do that in our next weeks triage session.
Comment 3•1 year ago
|
||
Set release status flags based on info from the regressing bug 1873916
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 5•1 year 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•1 year 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•1 year 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•1 year ago
|
Assignee | ||
Updated•1 year ago
|
Updated•1 year ago
|
Comment 10•1 year ago
|
||
Set release status flags based on info from the regressing bug 1873916
Assignee | ||
Comment 11•1 year 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•1 year ago
|
||
Assignee | ||
Comment 13•1 year 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•1 year ago
|
Assignee | ||
Comment 14•1 year ago
|
||
The upstream PR for Puppeteer is: https://github.com/puppeteer/puppeteer/pull/11794
Assignee | ||
Comment 15•1 year 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•1 year ago
|
||
A new Puppeteer release is out. That means we can push the stack now.
Comment 17•1 year ago
|
||
Comment 18•1 year ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/539c2d1bb3b6
https://hg.mozilla.org/mozilla-central/rev/d4f5993f9b5c
Comment 19•1 year 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•1 year ago
|
||
This should as best ride the trains, so lets not uplift.
Assignee | ||
Updated•11 months ago
|
Description
•