1847617 - Closing a window with a single about:newtab tab with no history results in a recently closed window entry

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect, P1)

Description

[Sam Foster [:sfoster] (he/him)]

STR:

• Open a new browser window. It only tab should be the New Tab page (about:newtab).
• Close the browser window -without having typed anything in the New Tab page, or navigated it

Expected:

• This is considered a "empty" window with no useful state to save, so no new entry in the History > Recently Closed Windows list is created

Actual

• A new Recently Closed Windows menu item is created, with "New Tab" as its label

In SessionStore.sys.mjs maybeSaveClosedWindow has the logic where we determine if there is useful state/history to save when closing a window. It evaluated each of the currently-open tabs in the window against this._shouldSaveTabState(), which has a list of URLs we consider not worth keeping.

As the window is closed, we handle a domwindowclosed observer notification, and call this.onClose, which in turn calls maybeSaveClosedWindow. At this point the tab's entries array is empty and we return false to not save the window data.

maybeSaveClosedWindow is called again when the promise from TabStateFlusher.flushWindow resolves. The tabData gets updated from data from cache, and now the tab.entries[0].url is about:home. about:home is not in the list in _shouldSaveTabState, so we return true from there and maybeSaveClosedWindow, so the window's state is saved into this._closedWindows - which is the collection used to populate the "Recently Closed Windows" lists.

Comment 1

[Sam Foster [:sfoster] (he/him)]

I'm blocking bug 1833522 on this, as its hard to talk about saved windows and tabs when the current behavior is already inconsistent. With the way the code and comments are written, I wonder if this used to work as expected and regressed at some point?

Comment 2

[Sam Foster [:sfoster] (he/him)]

No luck so far getting a regression range here. The _shouldSaveTabState function was added in Bug 581937, but its behavior depends on what the initial tab URL is: about:home vs. about:newtab. If this bug is specifically about windows with a sole about:newtab tab, then about:newtab seems to crash in builds I've tried from ~2017-18 which makes it harder to track down a specific regression. However, as I pointed out in comment #0, the final state of the single, initial tab that displays the New tab page actually resolves to about:home.

If you open a new window, use the "+" button to open a new tab at about:newtab, and close the initial tab, then close the windows leaving the 2nd New Tab tab open, we get the expected result of no saved window and new Recently Closed Windows entry. So I think perhaps the simple answer to this bug is to include about:home in the list of URLs in _shouldSaveTabState which we consider empty/not-worth-keeping.

Comment 3

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1847617 - Add about:home to the list of URLs we filter out when deciding if a closing window has state that should be saved. r?dao!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4ab6eb349233
Add about:home to the list of URLs we filter out when deciding if a closing window has state that should be saved. r=dao,sessionstore-reviewers

Comment 5

[Cristian Tuns]

Backed out for causing bc failures in /browser_aboutHomeLoading.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/components/urlbar/tests/browser/browser_aboutHomeLoading.js | Test timed out -

Comment 6

[Sam Foster [:sfoster] (he/him)]

Thanks, I wonder why this didn't show up in https://treeherder.mozilla.org/jobs?repo=try&revision=b66d102e353ec3fc37eca4fa952d731816141517

Comment 7

[Rob Lemley [:rjl]]

Attachment: WIP: Backed out changeset 4ab6eb349233 (Bug 1847617) for causing bc failures in /browser_aboutHomeLoading.js CLOSED TREE

Comment 8

[Daniel Bodea [:danibodea]]

I have attempted to investigate if this behavior is a regression, but it appears not. I've tested builds as old as Nightly v34.0a1 using the following steps:

1. launch the browser with a new profile
2. open a new window
3. close it (without any navigation done in it)
4. go to hamburger menu / library(new design only) / History / Restore Closed Windows
   Observe: New tab OR Nightly Start Page is displayed and it can be restored.
   Important to notice is that it won't be displayed in the Library window.

This does not reproduce in Nightly v26.0a1, probably because there was no option to open a new window, only to open a new tab or a private window; a detached tab (to create a new window) will not be shown in the recently closed windows section unless a webpage was loaded. THis behavior will still not reproduce in Nightly v118.0a1. THis investigation was done in WIndows 10.

Considering the above information, we can consider this as NOT a regression, but present since the implementation of the option to open a new window.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a42535df0242
Add about:home to the list of URLs we filter out when deciding if a closing window has state that should be saved. r=dao,sessionstore-reviewers

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/a42535df0242

Comment 11

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

The patch landed in nightly and beta is affected.
:sfoster, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox117 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

[Sam Foster [:sfoster] (he/him)]

(In reply to BugBot [:suhaib / :marco/ :calixte] from comment #11)

The patch landed in nightly and beta is affected.
:sfoster, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox117 to wontfix.

No, this is a long-standing bug I don't think we need to uplift.