Closed Bug 572889 Opened 12 years ago Closed 12 years ago

Move TabCandy out of a tab and into a per-window xul:deck

Categories

(Firefox Graveyard :: Panorama, defect, P1)

x86
macOS
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aza, Assigned: raymondlee)

References

Details

(Whiteboard: tracked)

Attachments

(1 file)

TabCandy is currently in a hidden tab in position #1. This needs to change.
Whiteboard: tracked
Some background info.

From Ian:
-----------------

Edward,

You posted these questions on the EtherPad:

Mardak: is there a bug for the xul:deck vs tab vs something else?
Mardak: if it's a deck, i believe there cant be animations. it'll just switch from browser to tabcandy

I don't think there's a bug yet... perhaps we should start one? 

Are you referring to the zoom in and out animations? Those are fake anyway... we switch to TabCandy and then animate a proxy. 

Incidentally, Neil Deakin had this to say:

The likely option is to create a separate tab, but where the tabpanel doesn't contain a <browser> as with the other tabs but contains what content you need. The tabstrip itself can be easily collapsed when needed and you could hide or collapse the specific tab if needed.

A separate deck element is possible, but the tabpanels are already a deck, so it may be better to use the tabpanels already present. Also, adding another deck means that additional 'view' objects are created which are low-level objects that were the old way of doing painting/transparency/etc. Creating less of them is probably ideal.

I understand you're experimenting with the possibilities... how's that going?

---------------



From Ed:
---------------
> A separate deck element is possible, but the tabpanels are already a deck,
> so it may be better to use the tabpanels already present.
Yeah, this might be a better idea now that I've been experimenting
with a separate xul:deck around the rest of the browser. There'll need
to be plenty of code changes either way of reusing the tabpanel or
switching to a separate deck.

I ran into some issues with the xul:deck where existing code assumes
tabcandy to be in a tab, so this affects lookup, events and
properties:
- various utils look for the 'active tab' to be tabcandy, but in a
deck/browser, it's its own separate world
- switching to tabcandy in a browser doesn't have the normal TabSelect
event triggered
- tabcandy in a xul:browser isn't the same as a Tab wrapped xul:tab

Reusing the tabpanel deck and keeping tabcandy as a special tab would
require some reworking of tabpanel to not expose tabcandy as a normal
tab, so switching to next/prev tabs don't select it and APIs don't
return it with regular tabs.

FYI, I was experimenting by modifying the bundled Minefield chrome
files to add in the xul:deck/browser pointed to the add-on's chrome
paths.

Ed
Assignee: nobody → raymond
Yeah, after thinking about this some more, I agree with Dão that tabcandy should just exist in its own browser. A bunch of the utils/tabs/etc. wrappers in js/core will need to be removed/inlined/refactored anyway.

From what I recall, there's various assumptions of tabcandy living in a tab, so switching to that tab sends out normal tab notifications. Also, there's bits that check if the currently selected tab is tabcandy would need to just check if tabcandy is displayed.

Raymond, what do you have for this already? I started hacking on some stuff a while back by modifying mozilla-central-side code and some stuff from the add-on. But eventually this would all be part of a patch against mozilla-central.
Priority: -- → P1
AFAIK, <xul:browser> seems to be <xul:iframe> with more stuff, like navigation, which we will neither need nor want to be enabled.

My initial naive approach has been to wrap most of browser.xul (from right after the app button to right before the svg back button hack) in a xul:deck.
The xul:deck has two child nodes: the former being <xul:iframe transparent="true" src="chrome://tabcandy/content/index.html"/>, and the latter being the regular browser.xul stuff.
This has visually worked somewhat, with the exception of sizing/flex issues, but aero glass is totally borked in the iframe.

If you know how to get <xul:iframe transparent="true"/> to work nicely or know of a superior approach, please let me know.

I will update after giving it another shot when I have time.
From my quick prototype, I put this after the <script> tags:

+<deck flex="1" id="candydeck">
+<vbox flex="1">

And this just before </window>:

+</vbox>
+<browser id="tabcandy" src="chrome://tabcandy/content/candies/revision-a/index.html" flex="1"/>
+</deck>
The xul:deck approach is better.  However, it would affect extensions in the field which add their own elements to the Firefox UI using overlay because all elements inside <window / > would be moved inside a deck, hence, we need to ask extension developers to update their overlay files for this change.

Another less elegant approach is to append the browser/iframe for tabCandy to the window in the browser.xul.  When user switches to TabCandy, we hide the navigator-toolbox, browser, and browser-bottombox like what we are doing at the moment.

I don't think it's possible to make iframe or browser transparent to show the aero glass window because a chrome/web page is loaded into iframe or browser and the page always has a background color.  One possible option is to use a vbox/hbox (it is transparent) instead, move all the elements and code out of the index.html and run them in the vbox/hbox, however, it will require a lot of code changes.
(In reply to comment #5)
> When user switches to TabCandy, we hide the navigator-toolbox, browser, and
> browser-bottombox like what we are doing at the moment.
TabCandy doesn't hide the browser as it's using it to display the TabCandy tab. I'm not sure what oddness might happen if you hide the main browser. Hiding the browser would actually remove it from the layout because we can't overlap browser with another browser/iframe I believe..

Extensions should be overlaying certain nodes+id pairs, so overlays shouldn't break.. ? And if they're going by the <window>.childNodes to get to browser, they're probably doing something wrong.
(In reply to comment #6)
> TabCandy doesn't hide the browser as it's using it to display the TabCandy tab.
> I'm not sure what oddness might happen if you hide the main browser. Hiding the
> browser would actually remove it from the layout because we can't overlap
> browser with another browser/iframe I believe..
I don't think it's a problem to hide the browser because it still exists, just not visible to users.  Let me give it a try. 

> 
> Extensions should be overlaying certain nodes+id pairs, so overlays shouldn't
> break.. ? And if they're going by the <window>.childNodes to get to browser,
> they're probably doing something wrong.
It's probably not an issue.
Blocks: 574217
Combined raymond's 2 deck-1 deck-2 patches into one.
Attachment #454597 - Flags: review?(ian)
Attachment #454597 - Flags: review?(ian) → review+
Comment on attachment 454597 [details] [diff] [review]
combined deck-1/-2

Looks good. The proof will be in the pudding (does it work when it all goes together), and we'll want Raymond to review as well.
http://hg.mozilla.org/users/edward.lee_engineering.uiuc.edu/tabcandy-central/rev/9cd7341890b8
Update code for tabCandy in xul:deck to not assume it's living in a tab and update various events and callbacks. 

Landed the "add-on side" of things.
http://hg.mozilla.org/users/edward.lee_engineering.uiuc.edu/tabcandy-central/rev/22aa5d167e1a
Wrap the browser in a deck with an iframe for tabcandy.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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: -- → ---
Version: unspecified → Trunk
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.