Open
Bug 1455264
Opened 7 years ago
Updated 10 months ago
Consolidate logic for owner/relatedTab/relatedToCurrent/aOpenerBrowser for tabs
Categories
(Firefox :: Tabbed Browser, enhancement, P2)
Firefox
Tabbed Browser
Tracking
()
NEW
People
(Reporter: Gijs, Unassigned, NeedInfo)
References
(Blocks 4 open bugs)
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).
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)
|
||
Updated•7 years ago
|
|
Reporter | |
Comment 1•7 years ago
|
||
Adding a note about tab insertion order at the bottom of the user story given bug 1396375.
User Story: (updated)
|
||
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•