580937 - Use plain xul:tabs instead of wrapping with BrowserTab

Status: RESOLVED FIXED

(Firefox Graveyard :: Panorama, defect)

Description

[Ed Lee :Mardak]

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.)

Comment 1

[Ed Lee :Mardak]

Attachment: helper patch to track down BrowserTab property uses

I'm using this to track down where BrowserTab properties are being used. And immediately ran into bug 580954.

Comment 2

[Ed Lee :Mardak]

Attachment: WIP v1

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.

Comment 3

[Ian Gilman [:iangilman]]

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 4

[Ian Gilman [:iangilman]]

Comment on attachment 459599 [details] [diff] [review]
WIP v1

Looks good to me. I assume the ultimate goal is to have TabItem.tab 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?

Comment 5

[Ed Lee :Mardak]

Attachment: WIP v1 cleanup

In addition to WIP v1.. cleaning up RAW to just plain tabs.

Comment 6

[Ed Lee :Mardak]

http://hg.mozilla.org/users/edward.lee_engineering.uiuc.edu/tabcandy-central/rev/4d2195e047af
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.

Comment 7

[(no longer active)]

Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy.  Filter the bugmail spam with "tabcandymassmove".