1974652 - Remember the order of tabs in a group after a close and then open

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect, P1)

Description

[johan.lorne]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:140.0) Gecko/20100101 Firefox/140.0

Steps to reproduce:

Save and close a group. Then reopen the group. The tabs are not in the order you left them.

Actual results:

The tabs are not in the order you left them.

Expected results:

The tabs should be in the order you left them.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Firefox for Android::General' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[qsjt0nvb05it]

I have the same problem on Linux 140.0.2

Comment 3

[Anders Gade]

I have the same issue on Firefox 140.0.4 (64 bit) Mozilla Firefox Snap for Ubuntu
canonical-002 -1.0

Comment 4

[Gabriel Astorgano[:astor]]

Attachment: Bug 1974652 Save tab group tab in order r=mak

Comment 5

[Stephen Thompson [:sthompson]]

Moving to the Session Restore component since saved tab groups are handled in there.

I thought that saving + closing tab groups saved the tabs in tab strip order, and I thought that restoring a saved tab group restores them in the same order. If that is not happening, that's a bug.

Comment 6

[Stephen Thompson [:sthompson]]

I could not reproduce with a quick check on macOS 142.0a1. Based on the reports, I'm going to mark this as being specific to Linux for now.

Comment 7

[Gabriel Astorgano[:astor]]

Attachment: Bug 1974652 Test tab group preserves order r=sthompson

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e9e24e5bb59b
https://hg.mozilla.org/integration/autoland/rev/da37575a15c6
Save tab group tab in order r=sessionstore-reviewers,sthompson

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/87a2627ffa2d
https://hg.mozilla.org/integration/autoland/rev/71b1a86b0484
Test tab group preserves order r=sthompson,sessionstore-reviewers

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b53de68f64cb
https://hg.mozilla.org/integration/autoland/rev/0219e9a2af35
Revert "Bug 1974652 Test tab group preserves order r=sthompson,sessionstore-reviewers" for causing bc failures @ browser_tab_groups_restore_closed_many_tabs.js

Comment 11

[amarc]

Backed out for causing bc fails @ browser_tab_groups_restore_closed_many_tabs

• Backout link
• Push with failures
• Failure Log

Comment 12

[Gabriel Astorgano[:astor]]

:amarc I can't seem reproduce the bug locally on linux.

The fail is related to the expanded tests that checks if the reported bug is fixed. Clearly I missed something.
Without the patch D257776 the bug is easy to reproduce and fails in both of the expanded tests. They test the tab order of a group after undoing a delete and when restoring a saved group. Treeherder is reporting an intermittent fail in the former on linux asan.

I'll see if I can reproduce the new issue and patch it.

Comment 13

[Stephen Thompson [:sthompson]]

:astor Please let me know if you'd like any support in debugging an intermittent test failure in Linux ASan builds. The linked test failure shows two pairs of adjacent tabs have their positions swapped. That's a failure mode I really wouldn't expect... There are some suggestions in https://firefox-source-docs.mozilla.org/testing/debugging-intermittents/index.html that might be helpful.

Comment 14

[Gabriel Astorgano[:astor]]

:sthompson Thanks for the link. Using --run-until-failure on headless mode I could reproduce the issue after a while. Now I just need to figure out the issue

Comment 15

[Jon]

on Windows 11 v24H2 Build 26100.4652

Firefox 141.0 (64-bit

I've found the tabs within a group do not re-open in the same order, the group opens tabs in seemingly random order.

"save and close group" doesn't retain tab order.

Comment 16

[mikael]

See on two computers:
Windows 11 Pro 24H2 10.0.26100 Build 26100
Firefox 141.0 (64-bit)

Tabs in a tab group does not always open in the same order as when saved.

• The reopened order often matches.
• Sometimes it seems to be somewhat random.
• A common pattern is that a number of tabs in the beginning is moved last (ABCED -> CEDAB).

Comment 17

[Stephen Thompson [:sthompson]]

Thanks for the additional reports for Windows!

A fix for this issue landed for Firefox 143 (release date September 16, 2025). I'll request that this fix be uplifted to Firefox 142 (release date August 19, 2025) so that it's available sooner.

The automated test for this fix failed to land. I'm taking a look at the tests to see if there's anything I can do to shore them up.

Comment 18

[Stephen Thompson [:sthompson]]

Attachment: Bug 1974652 Save tab group tab in order r=mak

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

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Are we still trying to land the test patch?

Comment 20

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: Low/Medium. Tab groups would continue to be restored in some cases with the tabs in a different order than they originally appeared in the tab strip.
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: Create a tab group with many tabs, "save and close" the tab group, then reopen it from the "list all tabs" menu. If fixed, the order of the tabs in the group should be the same before "save and close" and after reopening.
• Risk associated with taking this patch: Low
• Explanation of risk level: Simple logic change that only applies for tab groups. The tab group-specific code added is more explicitly correct.
• String changes made/needed: No
• Is Android affected?: no

Comment 21

[Dianna Smith [:diannaS]]

If we only plan on uplifting the patch and not landing the test patch (https://phabricator.services.mozilla.com/D257916), can we abandon that revision?

Comment 22

[Gabriel Astorgano[:astor]]

The test patch aims to check that restoring closed tabs preserves the tab group order. It intermittent fails on those checks, generally on a pair of tabs. I haven't been able to fix this. I would need help on it.

I think we need the test and solve this issue. Otherwise people could run into the problem with some of their tabs shuffling and then it would be harder to reproduce without someone making a test again.

Comment 23

[Stephen Thompson [:sthompson]]

Attachment: WIP: Bug 1974652 - tab group session order test fixes

Comment 24

[Stephen Thompson [:sthompson]]

I tried out the tests today in a local Linux ASan build. I also experience the intermittent failure after many test re-runs.

It's much easier to trigger the issue when running the closed group test in chaos mode (MOZ_CHAOSMODE=1 environment variable). It looks to me like the issue is triggered by a missing session state flush before closing the tab group. When the tab group wants to close, SessionStore calls maybeSaveClosedTab on each of the group's tabs; if any of those tabs' session state haven't received the URL that they loaded, then they will not be inserted into the closed tabs array immediately. When those tabs' browsers shut down a moment later, the SessionStore observes them shutting down and basically reruns the maybeSaveClosedTab, which will append the tab at that time. That asynchronicity can lead to incorrect ordering in the test. Flushing the session state before closing the tab group ensures that all of the tabs have their content reflected in session state so that they all get their data added synchronously to the closed group tabs list.

In actual user practice, the session is flushed every 15 seconds at most, so it's unlikely that this behavior manifested this bug. The root cause of this bug was most likely the use of the tab closing time from Date.now() to order the tabs. All of the tabs in a group are effectively closed at the same time, so there is room for problems. The fix that :astor already landed should have solved that issue. From my test runs, I've only encountered anomalous behavior from session state staleness in the test harness, and I'm reasonably confident that I have a fix for the test.

I published a WIP patch with proposed fixes for the tests on top of :astor's patch https://phabricator.services.mozilla.com/D260089. I also started a try run of the test changes. https://treeherder.mozilla.org/jobs?repo=try&revision=008dd9025bdaac00ce2e2d5a7f69b7be247123b1

Comment 25

[Dianna Smith [:diannaS]]

Should we not uplift this to beta and wait for this change? Might be better to have these go together

Comment 26

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

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

Comment 27

[Stephen Thompson [:sthompson]]

:astor My patch appears to result in a clean try build. Would you be able to apply my fixes in https://phabricator.services.mozilla.com/D260089 onto your patch https://phabricator.services.mozilla.com/D257916? If you're able to update your patch soon, then I can queue your patch to land in Nightly and then request Beta uplift for both your fix patch and the test patch.

Comment 28

[Gabriel Astorgano[:astor]]

Thanks :sthompson for the help! I'll get to it

Comment 29

[Stephen Thompson [:sthompson]]

Thank you so much :astor for taking the time! I queued your patch to land.

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/43e7ffd0ea08
https://hg.mozilla.org/integration/autoland/rev/0116994437df
Test tab group preserves order r=sthompson,sessionstore-reviewers

Comment 31

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/0116994437df

Comment 32

[Stephen Thompson [:sthompson]]

Attachment: Bug 1974652 Save tab group tab in order r=mak

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

Comment 33

[Stephen Thompson [:sthompson]]

Attachment: Bug 1974652 Test tab group preserves order r=sthompson

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

Comment 34

[Stephen Thompson [:sthompson]]

Beta uplift request for the fix patch: https://phabricator.services.mozilla.com/D260041
Beta uplift request for the associated automated test: https://phabricator.services.mozilla.com/D260335

Comment 35

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: None
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: This is a test-only change.
• Risk associated with taking this patch: Low
• Explanation of risk level: Test-only patch to protect against regressions from the associated fix in bug 1974652
• String changes made/needed: No
• Is Android affected?: no

Comment 36

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/cf98ca33f10e
https://hg.mozilla.org/releases/mozilla-beta/rev/d0dd5b149779
https://github.com/mozilla-firefox/firefox/commit/3bf161f6f71a
https://hg.mozilla.org/releases/mozilla-beta/rev/c9cb18e2edbf