597713 - Figure out how the Navigation Bar should behave during zoom transition

Status: RESOLVED WONTFIX

(Firefox Graveyard :: Panorama, enhancement, P4)

Description

[Hans Schmucker]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100917 Firefox/4.0b7pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100917 Firefox/4.0b7pre

Right now, entering and leaving TabCandy looks pretty strange since the content of the thumbnail does not match the content of the area it overlays during zoom. The thumbnail contains the content, but the area includes the navbar and other chrome. So, the Chrome has to move out of the way somehow. Fading it out might prove difficult since it's merged with the OS window background, but an easy way out would be overlaying it:
Make the TabCandy window start out overlaying the content area, then smoothly resize it to cover the browser chrome as well.

Reproducible: Always

Comment 1

[Michael Yoshitaka Erlewine [:mitcho]]

I like this idea, but I'm going to say we need to punt this to the future.

Comment 2

[Hans Schmucker]

I can make a quick attempt at this this weekend and I'd say there's a fair chance I'll get it done. If I could, would that be enough to still make it into FF4?

Comment 3

[Michael Yoshitaka Erlewine [:mitcho]]

(In reply to comment #2)
> I can make a quick attempt at this this weekend and I'd say there's a fair
> chance I'll get it done. If I could, would that be enough to still make it into
> FF4?

Sure, that'd be great! A patch would definitely be considered: the "Future" target is for our team's use, just to prioritize our work for the release.

Comment 4

[Hans Schmucker]

Attachment: Modified zoomIn method with padding and aspect ratio

Attached is a modified zoomIn method that adds a padding to the top that's about equivalent to tabbar+navbar on Windows as well as keeping the aspect ratio intact. It's not really useful on its own, but it really looks a whole lot better.

Comment 5

[Hans Schmucker]

Michael, could you give me a hint where the normal FF interface gets hidden? I can add the margin no problem, I can animate it, but I can't figure out how to keep the normal interface from disappearing until the animation is done. browser.js's _initFrame seems to be the one called when the TabCandy button is pressed (at least the key handler for Ctrl+Space seems to just call show, which does nothing but call _initFrame), but I just can't find the point where the normal interface gets hidden.

Comment 6

[Hans Schmucker]

Forget it... it really is a deck containing either the TabCandy interface or the browser. So either I need to seriously change the structure or do some hackery... maybe draw the TabCandy interface into an iframe overlaying the browser. Or not. Tricky.

Comment 7

[Hans Schmucker]

OK, so I can just replace the stack with a deck and change every single instance of selectedIndex use or, and this is what I would prefer (but I have no idea if this is allowed within the coding guidelines of this project), create a TabViewDeck class that wraps the stack element and has getters and setters emulating selectedIndex and whatever else is needed. Would that be alright or too hackish?

Comment 8

[Hans Schmucker]

After some trial and error it seems like I can easily exchange the deck for a stack and just leave the selectedIndex property as a way for TabCandy to track which one is currently open, while adding gTabViewDeck.lastElementChild.style.visibility="hidden"/"visible" to do the real work. Problem is: I can't really hide it (hidden=true) since this causes TabCandy to fail, so this could become potentially become a performance issue.

Comment 9

[Hans Schmucker]

OK, got a working implementation, but it's a little on the slow side, so I'm switching to Canvas-based navbar drawing for better performance.

Comment 10

[Hans Schmucker]

Attachment: Patch adding Canvas based zooming and NavBar transitions

This is a first implementation using Canvas. It also contains new zoom code since the two work hand in hand (there's still a little bug left where it doesn't get the correct thumbnail dimensions since the prepZoom method apparently overwrites the thumb's dimensions, not just the inner size of the canvas, but that's a different issue). It's diffed against today's (20100919031812) omni.jar, since I don't have a build setup right now, but it should be easy to port over if it's otherwise OK-ish.

One issue: On Aero (or any other platform with a transparent window background) the Canvas rendering of the navbar doesn't contain the background, meaning that the TabView iframe shines through. Wouldn't be much of an issue if the iframe background was also transparent (since the top few pixels mainly consists of shadows), but right now it isn't (see Bug #591652). I'll post a screenshot in a second.

It's just tested on my Win7 box with and without Aero so far, so testing is minimal and the code probably needs quite a few structural changes (if somebody could point me to the TabCandy coding guidelines, please), but at least it works and provides a starting point.

Comment 11

[Hans Schmucker]

Attachment: Screenshot of NavBar ontop of TabCandy BG

Comment 12

[Hans Schmucker]

Attachment: Video showing the effect

Comment 13

[Hans Schmucker]

OK, just discussed everything with aza and iangilman and the current prototype and all its problems are pretty much moot now. We want to fix the transition, but another way: Make the NavBar zoom along with the thumb, then clip it away either during or at the end of the transition. The codepath is pretty similar so I hope I can make it work tomorrow (at least a preliminary version).

Comment 14

[Michael Yoshitaka Erlewine [:mitcho]]

Comment on attachment 476788 [details] [diff] [review]
Patch adding Canvas based zooming and NavBar transitions

Thanks for the patch Hans. The video was quite impressive. It looks like you're going to keep working on this, so I just skimmed the code. A few things stand out, mostly in terms of code style:

1. your patch is against the "compiled" tabview.js file. This is actually the concatenation of a number of files in mozilla-central, so it would be hard (though not impossible) to accept such a patch. If you're not sure how to make a patch against mozilla-central, ask one of us on IRC. There are also tips on https://wiki.mozilla.org/Firefox/Projects/TabCandy/Work#Development
2. Your spacing seems inconsistent. Please make sure to provide adequate indentation. here are some guidelines: https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide
3. By abandoning .animate, you're introducing a lot of new canvas code here both in zoomIn and zoomOut. I wonder if maybe you could write a shared utility method to TabItem which can abstract some of these routines?

Comment 15

[Hans Schmucker]

Thanks Michael. I'm still experimenting and that's a whole lot more comfortable against the compiled version :) . When the testing-ready version comes I'll port it over to the source files, but right now this patch is only so you can have a quick test run.

About 2:
Yes, I'll need to adapt to that. I usually work with tab, not space-indention and no spacing inside lines (a=b+c instead of a = b+c). But I'll put this off until everything works.

About 3: Definitely. I was only experimenting on zoomIn and simply did a quick copy/paste job to add zoomOut so that I could see the speed on both.

I'll work on the next revision tonight. Let's see how it goes.

Comment 16

[Ian Gilman [:iangilman]]

In terms of low-end machines, it's hard for me to believe that canvas would be faster for the most simple option: a zooming bounding box. I imagine a transparent div with a black border would animate faster on low-end machines. Would be good to have real numbers though, I suppose.

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

Blocking on final as I originally did in bug 586486 - the issue we want to resolve is the "flip" that happens at the end of the current transition, where the content snaps into the space beneath the navigation toolbar. That's not cool.

Comment 18

[Ian Gilman [:iangilman]]

Sean, I realize this is a pretty open-ended bug. If nothing else, we need to attend to comment 17.

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

I reconsider my blocking assertion; what we have now will do fine.

Comment 20

[Kevin Hanes]

bugspam (moving b9 to b10)

Comment 21

[Kevin Hanes]

bugspam (removing b9)

Comment 22

[Kevin Hanes]

Punting unless Sean feels strongly about this bug.

Comment 23

[Ian Gilman [:iangilman]]

bugspam: returning Sean's bugs to the pool

Comment 24

[Justin Dolske [:Dolske]]

Panorama has been removed from Firefox 45, currently in Beta and scheduled for release on March 7th. As such, I'm closing all existing Panorama bugs.

If you are still using Panorama, you will see a deprecation message in Firefox 44, and when 45 is released your tab group data will be migrated to bookmarks, with a folder for each group. There are also a few addons offering similar functionality.

See https://support.mozilla.org/en-US/kb/tab-groups-removal for more info.

We're removing Panorama because it has extremely low usage (about 0.01% of users), and has a large number of bugs and usability issues. The cost of fixing all those issues is far too high to justify, and so we'll instead be focusing our time and energy on improving other parts of Firefox.