Use plain xul:tabs instead of wrapping with BrowserTab

RESOLVED FIXED

Status

RESOLVED FIXED
8 years ago
3 years ago

People

(Reporter: Mardak, Assigned: Mardak)

Tracking

Dependency tree / graph

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
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.)
(Assignee)

Comment 1

8 years ago
Created attachment 459356 [details] [diff] [review]
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.
(Assignee)

Updated

8 years ago
Blocks: 580952
(Assignee)

Updated

8 years ago
Depends on: 581078
(Assignee)

Comment 2

8 years ago
Created attachment 459599 [details] [diff] [review]
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.
Assignee: nobody → edilee
Attachment #459356 - Attachment is obsolete: true
Status: NEW → ASSIGNED
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 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?
Attachment #459599 - Flags: feedback?(ian) → feedback+
(Assignee)

Comment 5

8 years ago
Created attachment 459721 [details] [diff] [review]
WIP v1 cleanup

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

Comment 6

8 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Blocks: 581730

Comment 7

8 years ago
Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy.  Filter the bugmail spam with "tabcandymassmove".
Component: TabCandy → TabCandy
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.