Consolidate logic for owner/relatedTab/relatedToCurrent/aOpenerBrowser for tabs

NEW
Unassigned
(NeedInfo from)

Status

()

enhancement
P2
normal
a year ago
a month ago

People

(Reporter: Gijs, Unassigned, NeedInfo)

Tracking

(Depends on 1 bug, Blocks 4 bugs)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

User Story

1) what tab to select when a tab is closed. (currently called "owner")
2) what tab opened another tab, if any. (currently called "openerTab")
3) whether the opener tab is the currently selected tab (currently "relatedToCurrent")

There are a few issues with the current implementation:

1) "owner" is a bit of a weird term for something that (AFAICT) we only use to determine which tab to switch focus to. We also have a "_lastRelatedTabMap" that AFAICT is only used to reset the 'owner' property to null when this relationship stops existing. The names for these two things should match, and ideally be more clear about what we use the "owner" for, so maybe we could call it "nextTabAfterClosing" instead of "owner", and the map something similar ("nextTabAfterClosingMap" ?)

2) we have a bunch of logic relating (hah) to relatedtoCurrent and determining the opening tab (and/or "owner"). It can be passed explicitly, or we infer it from being passed an openerBrowser that is in the same window, or we infer it from being passed a referrer URI. Especially that last one seems fragile - what if (for instance) a link in the sidebar is used to open a new tab, then there ought to be a referring URI, but the tab ought not to be thought of as opened by the current tab. Separately, if we can pass an openerBrowser then we shouldn't need to be passing `relatedToCurrent` - we can just pass the browser reference directly. So for this, I think we should aim to:
a) stop using the referring URI logic
b) pass aOpenerBrowser when opening tabs instead of `relatedToCurrent`
c) remove the concept of `relatedToCurrent` as passing an opener browser provides more information anyway.

Note that the `relatedToCurrent` name also occurs in the prefs `browser.tabs.insertRelatedAfterCurrent`. The pref is true by default. If it's true, tabs that have an openerTab (right now, either openerBrowser, referring URI, or `relatedToCurrent`) will be inserted immediately after the opener (as opposed to at the end of the tabstrip).
(Reporter)

Description

a year ago
There are a few different concepts that the tabbrowser tries to keep track of for tabs that intersect in a few ways and can be confusing to grasp when first dealing with tabbrowser.js . We should try to make the code easier to follow. I'll give an overview in the "user story" field so that we can update it if my understanding is wrong in some way.

Dão, can you check my read of the code / issues, and confirm whether this seem like a reasonable plan? Feel free to edit the user story accordingly.
Flags: needinfo?(dao+bmo)
Depends on: 1238314
Priority: -- → P2
See Also: → 1449700
Blocks: 1396375
(Reporter)

Comment 1

a year ago
Adding a note about tab insertion order at the bottom of the user story given bug 1396375.
User Story: (updated)

Updated

a year ago
Blocks: 1459402
See Also: → 1445270
(Reporter)

Updated

a year ago
See Also: 1445270
Depends on: 1475606
See Also: → 1419947

Updated

6 months ago
Blocks: 1226546

Updated

5 months ago
Blocks: 1333837
You need to log in before you can comment on or make changes to this bug.