1873948 - "browsingContext.close" should be able to close the last tab of a window

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, defect, P2)

Description

[Treeherder Bug Filer]

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

[BugBot [:suhaib / :marco/ :calixte]]

:whimboo, since you are the author of the regressor, bug 1873916, could you take a look?

For more information, please visit BugBot documentation.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This needs a discussion. We will do that in our next weeks triage session.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1873916

Comment 4

[Intermittent Failures Robot]

5 failures in 3217 pushes (0.002 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• try: 5

Platform and build breakdown:

• macosx1015-64-qr: 2
  • debug: 2
• windows11-64-2009-qr: 1
  • debug: 1
• linux1804-64-qr: 2
  • debug: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1873948&startday=2024-01-08&endday=2024-01-14&tree=all

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1873948 - [webdriver-bidi] "browsingContext.close" should be able to close the last tab of a window.

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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
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

[Norisz Fay [:noriszfay]]

Backed out for causing Puppeteer related failure

Backout link

Push with failures

Failure log 1 // Failure log 2

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1873916

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1873948 - [puppeteer] Keep "browser.tabs.closeWindowWithLastTab" disabled for CDP.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The upstream PR for Puppeteer is: https://github.com/puppeteer/puppeteer/pull/11794

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(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.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

A new Puppeteer release is out. That means we can push the stack now.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
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

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/539c2d1bb3b6
https://hg.mozilla.org/mozilla-central/rev/d4f5993f9b5c

Comment 19

[BugBot [:suhaib / :marco/ :calixte]]

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 to wontfix.

For more information, please visit BugBot documentation.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This should as best ride the trains, so lets not uplift.