Closed Bug 1789652 Opened 3 months ago Closed 3 months ago

Firefox View - clicking on a recently closed tab opens a different tab

Categories

(Firefox :: Firefox View, defect, P1)

defect

Tracking

()

RESOLVED FIXED
106 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox-esr102 --- unaffected
firefox104 --- unaffected
firefox105 --- unaffected
firefox106 --- fixed

People

(Reporter: vchin, Assigned: sclements)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression, Whiteboard: [fidefe-2022-mr1-firefox-view])

Attachments

(1 file)

STR:

  1. Click on a recently closed tab

Outcome:
Focus switches to a completely unrelated tab and the tab disappears from the Recently Closed list

Expected Results:
The expected tab shows up

106.0a1 (2022-09-06) on MacOS

Worth noting that the list of tabs were closed via the tree style tab add-on.

This is likely not caused not caused by the addon. I'm able to reproduce on MacOS as well.

Severity: -- → S2
Priority: -- → P1
Whiteboard: [fidefe-2022-mr1-firefox-view]
Blocks: firefox-view

(In reply to Ray Fambro from comment #2)

This is likely not caused not caused by the addon. I'm able to reproduce on MacOS as well.

Can you provide steps to reproduce this? I cannot reproduce easily.

Flags: needinfo?(rfambro)
Summary: Clicking on a recently closed tab opens a different tab → Firefox View - clicking on a recently closed tab opens a different tab

Interesting. I was able to reproduce again in my existing FxView window where I had 13 old tabs in the "Recently closed" section (all urls accessed within the last 2 hours). I should note that I don't believe that I refreshed this window since yesterday when we were still working through some other bug fixes.

Upon opening a new window with blank "Recently closed" history just now, I'm no longer able to reproduce this issue.

Flags: needinfo?(rfambro)

I just reproduced this. Let me see if I can narrow down steps.

Flags: needinfo?(gijskruitbosch+bugs)
Assignee: nobody → sclements
Status: NEW → ASSIGNED

I've found a way to reproduce this reliably with a local build (ensure ./mach watch is running):
Start with a clean profile if you have a bunch of old tabs sitting around because its probably easier to see the issue, but I've found I don't need to do this every time to reproduce it.

  1. open 4 tabs
  2. close 3 tabs (leave 1 open, so you don't close the browser)
  3. look at fxview
  4. restart fxview with the mach watch shortcut (cmd+opt+r on mac)
  5. click on fxview
  6. click on a middle tab in the recently closed list
  7. close the newly opened tab before going back to fxview
  8. Go back to fxview. The newly closed tab doesn't show up.
  9. Try reopening another tab and the wrong tab opens; you might also see that the url has updated for domain but doesn't match the title

I think the bug is here. We're adding tabs to this.closedTabsData by newTab.closedID but the problem is that ID is not unique and multiple entries closed around the same time will have the same ID. So in that loop, we end up adding entries to this.closedTabsData (but only if a particular closedID doesn't exist) and in that same loop overwriting the urls of existing entries we've just added with erroneous tabsToUpdate.

(In reply to Sarah Clements [:sclements] from comment #6)

I think the bug is here. We're adding tabs to this.closedTabsData by newTab.closedID but the problem is that ID is not unique and multiple entries closed around the same time will have the same ID.

Uh, that sounds bad. https://searchfox.org/mozilla-central/rev/275630dfceb88eee07e4c7d38cd021dd39c2ab23/browser/components/sessionstore/SessionStore.jsm#2802 looks like it's monotonically increasing. But perhaps when using session restore and getting back tabs from the previous session, the IDs can conflict?

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to :Gijs (he/him) from comment #7)

(In reply to Sarah Clements [:sclements] from comment #6)

I think the bug is here. We're adding tabs to this.closedTabsData by newTab.closedID but the problem is that ID is not unique and multiple entries closed around the same time will have the same ID.

Uh, that sounds bad. https://searchfox.org/mozilla-central/rev/275630dfceb88eee07e4c7d38cd021dd39c2ab23/browser/components/sessionstore/SessionStore.jsm#2802 looks like it's monotonically increasing. But perhaps when using session restore and getting back tabs from the previous session, the IDs can conflict?

Yeah, looks like we just restart counting at 0. But the closed tabs from the previous session can also include items with the same closedId...

Regressed by: 1787565

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

Attachment #9294485 - Attachment description: WIP: Bug 1789652 - Overwrite closedIDs when restoring sessions r=Gijs → Bug 1789652 - Overwrite closedIDs when restoring sessions r=Gijs
Pushed by sclements@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d057e8cf08cf
Overwrite closedIDs when restoring sessions r=Gijs

Backed out changeset d057e8cf08cf (Bug 1789652) for causing bc failures on browser_closedId.js.
Backout link
Push with failures <--> bc2
Failure Log

Flags: needinfo?(sclements)
Flags: needinfo?(sclements)
Pushed by sclements@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4a17392e12c8
Overwrite closedIDs when restoring sessions r=Gijs
Blocks: 1789004
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 106 Branch
You need to log in before you can comment on or make changes to this bug.