1888841 - forgetClosedTabById() never getting passed ids due to a typo

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect, P1)

Description

[Andrew McCreight [:mccr8]]

onDismissTab in recentlyclosed.mjs contains the typo e.originalTarget.souceWindowId. It looks like this might mean that forgetClosedTabById will never get ids passed to it. It looks like this was introduced by bug 1845836.

Comment 1

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

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

:sfoster, since you are the author of the regressor, bug 1845836, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Sounds like we're missing some test coverage here then too.

Comment 3

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

The sourceWindowId that is typo'd here is used to retrieve the closed tab record associated with the window it was closed from. It is really an optimization. Without it we have to iterate through all the windows' closed tab data. With the identifier passed through from firefox view, we can resolve the reference faster.

From my testing, the feature is currently working even with the typo:

1. Open tabs in 2 browser windows
2. Close a tab in window 2
3. In window 1, open firefox view and find the tab you just closed in the "Recently closed tabs" list
4. Click the "dismiss" X button to forget this closed tab
5. In window 2, open the History > Recently closed tabs menu

ER:
The tab you closed in step 1 is not in this list

So, the impact here is relatively minor: it should only be noticeable in sessions with many windows and closed tabs.

Comment 4

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

Attachment: Bug 1888841 - Fix typo when passing sourceWindowId to SessionStore's forgetClosedTabById. r?#fxview-reviewers!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe3be4ea2372
Fix typo when passing sourceWindowId to SessionStore's forgetClosedTabById. r=fxview-reviewers,jsudiaman,kcochrane

Comment 6

[tszentpeteri]

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

Comment 7

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

For more information, please visit BugBot documentation.

Comment 8

[Bianca Hidecuti, Desktop Test Engineering [:bhidecuti]]

If following the steps from Comment 3 I can confirm that the tab closed in step 2 is not listed in the Recently closed tabs menu on the affected builds (on Firefox 124, Firefox 125.0.3 or on Firefox Nightly 126.0a1 build from 2024-04-02) using macOS 12.6.6, Windows 11 and Ubuntu 22.04.
I can also confirm that the tab closed in step 2 is not listed in the Recently closed tabs menu on Firefox 126.0-build2 on the previously mentioned platforms.
@sfoster, could you please let me know if something else should be verified here? Thank you in advance!

Comment 9

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

(In reply to bhidecuti from comment #8)

If following the steps from Comment 3 I can confirm that the tab closed in step 2 is not listed in the Recently closed tabs menu on the affected builds (on Firefox 124, Firefox 125.0.3 or on Firefox Nightly 126.0a1 build from 2024-04-02) using macOS 12.6.6, Windows 11 and Ubuntu 22.04.
I can also confirm that the tab closed in step 2 is not listed in the Recently closed tabs menu on Firefox 126.0-build2 on the previously mentioned platforms.
@sfoster, could you please let me know if something else should be verified here? Thank you in advance!

Yeah that is a good verification - we just needed to confirm nothing regressed.

Comment 10

[Bianca Hidecuti, Desktop Test Engineering [:bhidecuti]]

Thank you for the confirmation, sfoster!
Based on Comment 8, I am marking this verified as fixed.