Last Comment Bug 713287 - Animation for closing the last tab has a slight delay near the end when the tab bar overflows
: Animation for closing the last tab has a slight delay near the end when the t...
Status: RESOLVED FIXED
[snappy]
: polish
Product: Firefox
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 12
Assigned To: Dão Gottwald [:dao]
:
Mentors:
: 587842 708912 (view as bug list)
Depends on:
Blocks: 593680 715071
  Show dependency treegraph
 
Reported: 2011-12-23 13:00 PST by Mike Hommey [:glandium]
Modified: 2013-11-12 00:56 PST (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (909 bytes, patch)
2011-12-24 14:06 PST, Dão Gottwald [:dao]
mak77: review+
Details | Diff | Review
(Screenshot for comment 16 - this is the state when there's a pause) (3.23 KB, image/png)
2011-12-30 08:12 PST, mozbugz
no flags Details

Description Mike Hommey [:glandium] 2011-12-23 13:00:08 PST
STR:
- open plenty of tabs (enough to have the scroll buttons showing up)
- close the right-most tab
- in slow motion, what you would see is the animation happening (other tabs moving to the right) until some point at less than an icon size left to move, the animation stops for a few moments and then jumps to the final position.

This is not completely noticeable, but it so happens that on a firefox-as-xulrunner-app setup, the animation actually stops for a little while in the not quite finished position, and then the following message shows up:
ASSERT: Giving up waiting for the tab closing animation to finish (bug 608589)
Stack Trace: 
0:([object XULElement],[object XULElement],0)

But I think the latter is related to another error i get in the console related to telemetry failure on onxbltransitionend.

That error put aside, the slight delay can be seen on current (vanilla) beta and aurora (didn't test other builds)
Comment 1 Mike Hommey [:glandium] 2011-12-23 13:04:54 PST
I confirm the error message is due to telemetry.
Comment 2 Mike Hommey [:glandium] 2011-12-23 13:52:06 PST
Adding "return" there:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#3266

allows to see where the animation stops. That being the transitionend event suggests the transition does not actually cover the expected animation.
Comment 3 Marco Castelluccio [:marco] 2011-12-23 14:04:45 PST
Mike, there is bug 708912 that I've filed some time ago. We can close this or that as duplicate.
Comment 4 Mike Hommey [:glandium] 2011-12-23 14:45:11 PST
I think bug 708912 is a different issue. That one seems to be about the whole animation being slow. This one is that the animation is incomplete and then jumps to final position.
Comment 5 Marco Castelluccio [:marco] 2011-12-23 16:56:19 PST
(In reply to Mike Hommey [:glandium] from comment #4)
> This one is that the animation is incomplete and
> then jumps to final position.

This is what I meant with "animation deferred" :D. Closing that as this is more clear.
Comment 6 Marco Castelluccio [:marco] 2011-12-23 16:56:34 PST
*** Bug 708912 has been marked as a duplicate of this bug. ***
Comment 7 Marat Tanalin | tanalin.com 2011-12-24 08:25:50 PST
The error ("ASSERT: Giving up waiting...") periodically appears in Firefox 10b1 (build from http://hg.mozilla.org/releases/mozilla-beta/rev/c130e43aa4b5).

Exact steps to reproduce are unknown, but they are _different_ from steps described in the bug description (comment 0) which don't cause the error for me.
Comment 8 Dão Gottwald [:dao] 2011-12-24 14:06:26 PST
Created attachment 584229 [details] [diff] [review]
patch

The tab contents prevent the tab from shrinking down to zero. We can just completely hide that stuff, we don't really need the opacity transition.
Comment 9 Dão Gottwald [:dao] 2011-12-24 14:19:16 PST
I immediately saw the positive effect of this patch on Linux. On Windows, the transition was so choppy that it was hard to tell a difference, until I disabled hardware acceleration... Bug 633157 is quite bad.
Comment 10 Marco Bonardo [::mak] 2011-12-28 09:14:12 PST
Comment on attachment 584229 [details] [diff] [review]
patch

Review of attachment 584229 [details] [diff] [review]:
-----------------------------------------------------------------

Looking at the reason the animation was added, there should be no problem in testing this change live, and eventually re-evaluate later.
Is it noticeable that these contents appear at once when the tab opens? My system is likely too fast to notice the difference.
Comment 11 Dão Gottwald [:dao] 2011-12-28 10:13:37 PST
(In reply to Marco Bonardo [:mak] from comment #10)
> Is it noticeable that these contents appear at once when the tab opens? My
> system is likely too fast to notice the difference.

They don't appear immediately, since we also have a transition for the whole tab's opacity.
Comment 13 Marco Bonardo [::mak] 2011-12-29 03:32:11 PST
https://hg.mozilla.org/mozilla-central/rev/0e7bacdbb8fc
Comment 14 tabmix.onemen 2011-12-29 22:58:53 PST
(In reply to Dão Gottwald [:dao] from comment #8)
> Created attachment 584229 [details] [diff] [review]
> patch
> 
> The tab contents prevent the tab from shrinking down to zero. We can just
> completely hide that stuff, we don't really need the opacity transition.

what about tab borders?
why not to hide the borders to ?
Comment 15 Dão Gottwald [:dao] 2011-12-30 02:54:33 PST
(In reply to onemen.one from comment #14)
> (In reply to Dão Gottwald [:dao] from comment #8)
> > Created attachment 584229 [details] [diff] [review]
> > patch
> > 
> > The tab contents prevent the tab from shrinking down to zero. We can just
> > completely hide that stuff, we don't really need the opacity transition.
> 
> what about tab borders?
> why not to hide the borders to ?

I don't see that borders are a problem here. If you do, please file a bug.
Comment 16 mozbugz 2011-12-30 08:12:06 PST
I'm using a Win64 2011-12-30 nightly (0d684c34d1e4) which is supposed to have this fix, if I understand correctly, and I still get the problem, easily and every time.

Toggling HW acceleration on and off doesn't make any difference. The rest of the animations are smooth, but there's a very noticeable pause at the end of the tab closing one, when the closed tab has disappeared, before the empty space is removed.

The pause is long enough to make taking an screenshot easy; I'm attaching one.
Comment 17 mozbugz 2011-12-30 08:12:58 PST
Created attachment 584969 [details]
(Screenshot for comment 16 - this is the state when there's a pause)
Comment 18 Dão Gottwald [:dao] 2011-12-30 08:17:20 PST
(In reply to mozbugz from comment #16)
> I'm using a Win64 2011-12-30 nightly (0d684c34d1e4) which is supposed to
> have this fix, if I understand correctly, and I still get the problem,
> easily and every time.
> 
> Toggling HW acceleration on and off doesn't make any difference. The rest of
> the animations are smooth, but there's a very noticeable pause at the end of
> the tab closing one, when the closed tab has disappeared, before the empty
> space is removed.
> 
> The pause is long enough to make taking an screenshot easy; I'm attaching
> one.

Can you reproduce this in a new profile? If so, please file a bug on it.
Comment 19 mozbugz 2011-12-30 08:20:01 PST
No, I can't. Looks like it was an userChrome.css issue.

Sorry about the noise, I really should have checked that first.
Comment 20 tabmix.onemen 2012-01-03 23:06:33 PST
filled bug 715071 as a regression of this bug
Comment 21 Dão Gottwald [:dao] 2012-02-29 04:58:19 PST
*** Bug 587842 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.