543206 - Tab opening animation

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, enhancement)

Description

[Dão Gottwald [:dao]]

Attachment: patch

Comment 1

[Dão Gottwald [:dao]]

Attachment: patch v2

some minor tweaks

Comment 3

[Dão Gottwald [:dao]]

Attachment: patch v2

updated to current trunk

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 442705 [details] [diff] [review]
patch v2

>diff --git a/browser/base/content/browser.css b/browser/base/content/browser.css

>+.tabbrowser-tab[fadein] {
>+  -moz-transition: .2s min-width ease-out, .25s max-width ease-out;
>+}

Wouldn't it be enough to only transition the min-width property?

The tab title text being squished during the animation is a bit strange, perhaps we should transition it's opacity/visibility at the same rate?

>+.tabbrowser-tabs[overflow="true"] > .tabbrowser-tab[fadein] {
>+  -moz-transition: 1ms max-width; /* making sure we get the transitionend event */
>+}

Can you elaborate on the comment? I don't really understand this rule's purpose.

>diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml

>+      <handler event="transitionend"><![CDATA[

>+        if (tab.getAttribute("selected") == "true") {

>+          this._fillTrailingGap();
>+          this._handleTabSelect();

Is _handleTabSelect() needed here because the tab isn't necessarily the correct size when its TabSelect fires?
_fillTrailingGap also wasn't called by addTab before, so I'm not sure I understand why it's called here.

Comment 5

[Dão Gottwald [:dao]]

Attachment: patch v3

I've added a skipAnimation parameter to addTab in order to fix the "restore visible tabs first" feature.

(In reply to comment #4)
> (From update of attachment 442705 [details] [diff] [review])
> >diff --git a/browser/base/content/browser.css b/browser/base/content/browser.css
> 
> >+.tabbrowser-tab[fadein] {
> >+  -moz-transition: .2s min-width ease-out, .25s max-width ease-out;
> >+}
> 
> Wouldn't it be enough to only transition the min-width property?

Tabs take up as much space as they can, so min-width alone doesn't suffice.

> The tab title text being squished during the animation is a bit strange,
> perhaps we should transition it's opacity/visibility at the same rate?

done

> >+.tabbrowser-tabs[overflow="true"] > .tabbrowser-tab[fadein] {
> >+  -moz-transition: 1ms max-width; /* making sure we get the transitionend event */
> >+}
> 
> Can you elaborate on the comment? I don't really understand this rule's
> purpose.

Changed the comment to this: "When overflowing, new tabs are scrolled into view smoothly, which doesn't go well together with the width transition. The following disables the transition but ensures that tabbrowser gets the transitionend event."

> >diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml
> 
> >+      <handler event="transitionend"><![CDATA[
> 
> >+        if (tab.getAttribute("selected") == "true") {
> 
> >+          this._fillTrailingGap();
> >+          this._handleTabSelect();
> 
> Is _handleTabSelect() needed here because the tab isn't necessarily the correct
> size when its TabSelect fires?

Yes.

> _fillTrailingGap also wasn't called by addTab before, so I'm not sure I
> understand why it's called here.

It wasn't called inside of addTab, but it was called when the added tab was selected subsequently.

Comment 6

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/9d9078188333

I made tabbrowser.xml instead of browser.css skip the animation in the overflow case.

I still had to wrestle with a few tests, the most interesting outcome being http://hg.mozilla.org/mozilla-central/rev/33ff230a5b78 -- adjustTabstrip is only interested in the first tab's width, so letting it wait for the transition to end was pointless.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Dao, can you please give some details about this bug? It's really unclear what has been changed when reading the comments.

Also as it sounds there are automated tests in-place? If that's the case please remember to set the in-testsuite flag. It will help us a lot when verifying fixes and work on Litmus tests. For the latter case I would like to know what has been changed with this patch. Thanks.

Comment 8

[Paul [pwd]]

Can we slow down the animation and enable animation on on a full tab bar? IIRC there was a Test Pilot study that showed most tab bars are in fact full of tabs.

Comment 9

[Damon Sicore (:damons)]

Sorry if this is off-base, but when someone command-click's on a link to open a new tab with the content of that link, are we kicking off the request+load before the tab animation occurs and then doing the animation+load+parsing+etc in parallel?  Just curious as I would think that if those things were done in parallel it might improve the perception of snappiness.

Comment 10

[Dão Gottwald [:dao]]

(In reply to comment #9)
> Sorry if this is off-base, but when someone command-click's on a link to open a
> new tab with the content of that link, are we kicking off the request+load
> before the tab animation occurs and then doing the animation+load+parsing+etc
> in parallel?

Yes to both questions.

> Just curious as I would think that if those things were done in
> parallel it might improve the perception of snappiness.

The reason why this doesn't quite work right now is that anything happening on the main thread will halt the animation.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #7)
> Dao, can you please give some details about this bug? It's really unclear what
> has been changed when reading the comments.
> 
> Also as it sounds there are automated tests in-place? If that's the case please
> remember to set the in-testsuite flag. It will help us a lot when verifying
> fixes and work on Litmus tests. For the latter case I would like to know what
> has been changed with this patch. Thanks.

Dao, can you please give me an answer to my question from a week ago? We have a couple of failed Litmus test results, probably because of this change.

Comment 12

[Dão Gottwald [:dao]]

There's not much more to say than what's in the summary. New tabs fade in rather than appearing immediately. If you have a specific question, please be more concrete.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks. Would be great if your initial comment would be that informative in the future.

Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a6pre) Gecko/20100620 Minefield/3.7a6pre

Comment 14

[imradyurrad]

The animation's slightly off
http://www.stephenhorlander.com/images/blog-posts/tab-animation/new-tab-growth-frame-by-frame.png