Closed Bug 580937 Opened 12 years ago Closed 12 years ago

Use plain xul:tabs instead of wrapping with BrowserTab


(Firefox Graveyard :: Panorama, defect)

Not set


(Not tracked)



(Reporter: Mardak, Assigned: Mardak)




(2 files, 1 obsolete file)

BrowserTab provides various shorthands like .url, .favicon, etc. It also provides a way to get back to the xul:tab with .raw. We should probably just pass around the xul:tab and inline the sugar.

So this is getting rid of BrowserTab (and not TabItem or TabMirror; but those perhaps can get combined into a object hanging off of the xul:tab for another bug.)
I'm using this to track down where BrowserTab properties are being used. And immediately ran into bug 580954.
Blocks: 580952
Depends on: 581078
Attached patch WIP v1Splinter Review
Builds on the helper tracker patch and exposes a new .RAW property to do non-sugared stuff.

One thing I'm concerned about is that some places assume gBrowser contains the tab where in the future, a tab in tabcandy might not belong to the current gBrowser.
Assignee: nobody → edilee
Attachment #459356 - Attachment is obsolete: true
Attachment #459599 - Flags: feedback?(ian)
The main thrust of this bug sounds good to me. 

I don't know that it's a good idea to attach a TabItem to a xul:tab, though,
for these reasons: 

* TabItem should be just about the visual display of a tab in Tab Candy, and
shouldn't relate to the rest of the browser at all. 
* There's a lot to be said for keeping the TabItem code and its relatives
contained within the Tab Candy frame. Attaching a TabItem to a xul:tab would
cause all related code to leak out into the rest of the browser, right?
* Eventually we may have multiple Tab Candy frames (in multiple windows) all
pointing to the same tabs; at that point we would need the xul:tab to maintain
a list of all of the TabItems, which seems messy. 
As for gBrowser assumption; good catch... should certainly be fixed.
Comment on attachment 459599 [details] [diff] [review]
WIP v1

Looks good to me. I assume the ultimate goal is to have be the xul:tab, yes?

One question: the BrowserTab helper functions have a lot of checks to see if the various objects along the chain (say .linkedBrowser.currentURI.spec) all exist... since you've removed those checks, would you say they are unnecessary?
Attachment #459599 - Flags: feedback?(ian) → feedback+
Attached patch WIP v1 cleanupSplinter Review
In addition to WIP v1.. cleaning up RAW to just plain tabs.
Don't expose BrowserTab and have Tabs give sugarless xul:tabs that don't need .raw-ing. Remove most of the BrowserTab functionality only to leave the event bits.
Closed: 12 years ago
Resolution: --- → FIXED
Blocks: 581730
Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy.  Filter the bugmail spam with "tabcandymassmove".
Product: Mozilla Labs → Firefox
Target Milestone: -- → ---
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.