1952902 - Only 25 Tabs are restored for Saved and Closed or Deleted groups

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect, P1)

Description

[Rares Doghi, Desktop QA]

Attachment: 25s.mp4

Found in

• 138.0a1 (2025-03-10)

Affected versions

• 138.0a1 (2025-03-10)
  137.0b4

Affected platforms

• All

Steps to reproduce

1. Have at least 30 tabs.
2. Add all 30 tabs to a group.
3. Open a New tab.
4. Save and Close the Tab group or Delete it.
5. Restore the Tab group from Recently closed Tabs or from the List all Tabs menu

Expected result

• All tabs should be restored to the previously saved group

Actual result

• Only 25 tabs are restored to the previously saved group

Regression range
Not a Regression

Comment 1

[Stephen Thompson [:sthompson]]

My changes from bug 1933574 started running closed and saved-and-closed tab groups' tab data through maybeSaveClosedTab in order to utilize maybeSaveClosedTab's existing code for reformatting tab data into the same data format used by closed tabs. This had the unintended side effect of limiting the number of tabs we save in closed or saved-and-closed tab groups to the value of pref browser.sessionstore.max_tabs_undo, which defaults to 25.

Comment 2

[Stephen Thompson [:sthompson]]

Attachment: Bug 1952902 - closed tab groups and saved tab groups should retain all tabs r=dao,dwalker,jswinarton

When closing single or multiple tabs, session state has historically capped the number of closed tabs being tracked. The main motivation was to prevent session file sizes on disk from getting too large.

When we introduced tab groups to session state, we avoided those caps when the tab group itself is being closed/saved. Users think of tab groups as whole entities. We were successfully saving tab groups with any number of tabs.

I introduced a regression causing closed tab groups and saved-and-closed tab groups to have their tab lists in session state capped. This patch resolves the regression by bypassing the cap when a closing tab is being closed as part of a tab group closing as a whole. Single or multiple tabs that are closed directly by the user should still be capped even if those tabs were in a tab group.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b22a8fb3e4b
closed tab groups and saved tab groups should retain all tabs r=dao,sessionstore-reviewers

Comment 4

[amarc]

https://hg.mozilla.org/mozilla-central/rev/4b22a8fb3e4b

Comment 5

[Rares Doghi, Desktop QA]

This issue is verified as fixed in our latest Nightly 138.0a1 (2025-03-12).
Since the Tab group limit is no longer 25, I encountered a performance issue with a Tab group of over 200 tabs and I logged it here Bug 1953533.

Comment 6

[Stephen Thompson [:sthompson]]

Attachment: Bug 1952902 - closed tab groups and saved tab groups should retain all tabs r=dao,dwalker,jswinarton

When closing single or multiple tabs, session state has historically capped the number of closed tabs being tracked. The main motivation was to prevent session file sizes on disk from getting too large.

When we introduced tab groups to session state, we avoided those caps when the tab group itself is being closed/saved. Users think of tab groups as whole entities. We were successfully saving tab groups with any number of tabs.

I introduced a regression causing closed tab groups and saved-and-closed tab groups to have their tab lists in session state capped. This patch resolves the regression by bypassing the cap when a closing tab is being closed as part of a tab group closing as a whole. Single or multiple tabs that are closed directly by the user should still be capped even if those tabs were in a tab group.

Original Revision: https://phabricator.services.mozilla.com/D241087

Comment 7

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: Data loss for users who save and/or close tab groups with more than 25 tabs and then later restore those tab groups
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: Create more than 25 non-empty tabs. Multi-select all tabs, right-click, add tabs to a new tab group. Right click tab group label and choose to save and close the group. Open the tab overflow menu/all tabs menu and click on the tab group to restore it. The tab group should be restored to its previous condition with all tabs.
• Risk associated with taking this patch: Low
• Explanation of risk level: Single-line logical change to exempt tab groups from the standard limit on the number of closed tabs tracked by the session store. Automated tests confirm no regressions related to the limits on closed tabs. Automated tests confirm that users will have all grouped tabs retained in session state.
• String changes made/needed: No
• Is Android affected?: no

Comment 8

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/c4354a4b80c4

Comment 9

[Rares Doghi, Desktop QA]

Verified as fixed in our latest Beta 137.0b6