When opening a new tab it is transitioned by min- and max-width. The .tab-background however is immediately visible with its -15px margin and makes the new tab jump back a little before sliding forward again.

Here's a slow-motion video of the current tabopen animation. I added a border to visualize the tab bounds and slowed down the transitions. You can see that the new tab starts with its rounded borders immediately visible above the previous tab.

Here's a proposed patch that fades in .tab-background and makes the tabopen animation IMO look a lot smoother. Not sure who to ask for feedback on this. Feel free to redirect.

Here's the same slow-motion video with the improved tabopen animation to clarify the change. With the patch applied (in non-slow-mo mode) opening a new tab looks better to me now.

A really slow mo screencast : http://www.youtube.com/watch?v=6cw4ki2V58w

Comment on attachment 8334419 [details] [diff] [review] 0001-fade-in-.tab-background-to-improve-tabopen-animation.patch I'm happy to give you an f+ for this, although r+ should probably come from mconley or MattN. However, we should really run this through try and check if this regresses TART. :-)

Applying the transition to selected tabs, only.

Let's see what TART says: https://tbpl.mozilla.org/?tree=Try&rev=b3d38ee38099

Comment on attachment 8334453 [details] [diff] [review] 0001-Bug-940262-Fade-in-.tab-background-to-improve-tabope.patch >--- a/browser/base/content/browser.css >+++ b/browser/base/content/browser.css >@@ -114,16 +114,25 @@ tabbrowser { > .tabbrowser-tab:not([pinned]):not([fadein]) { > max-width: 0.1px; > min-width: 0.1px; > visibility: hidden; > transition: min-width 200ms ease-out, > max-width 230ms ease-out; > } > >+.tabbrowser-tab[selected]:not([pinned]) > .tab-stack > .tab-background { >+ opacity: 0; >+} >+ >+.tabbrowser-tab[selected][fadein]:not([pinned]) > .tab-stack > .tab-background { >+ opacity: 1; >+ transition: opacity 180ms ease-out 20ms; >+} tab-background inherits [selected] and [pinned]. It doesn't inherit [fadein], but it could. You should set opacity:0 in a rule using :not([fadein]) and avoid setting opacity:1 at all.

Looks like a regression to me, somewhat expected due to more work when fading in... http://perf.snarkfest.net/compare-talos/index.html?oldRevs=4bf55c6b9c25&newRev=b3d38ee38099&submit=true

This patch removes the fadein, after a short offset .tab-background is just shown - i.e. switched from visibility:hidden to visible. This looks probably even better? And should not regress TART? Let's see.

FTR, here are try builds for the previous patch, if anyone wants to try it out: https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ttaubert@mozilla.com-b3d38ee38099/

Try builds for the latest patch are here: https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ttaubert@mozilla.com-6843671acee4/ (Some platforms are still building.)

While OS X 10.7 is still running it pretty sure looks like the new patch doesn't regress anything: http://perf.snarkfest.net/compare-talos/index.html?oldRevs=9225b9430a51&newRev=6843671acee4&submit=true

Comment on attachment 8335161 [details] [diff] [review] 0001-Bug-940262-Hide-.tab-background-in-the-early-tabopen.patch Not exactly sure that those are the changes that dao had in mind but I figured I just ask for review.

Comment on attachment 8335161 [details] [diff] [review] 0001-Bug-940262-Hide-.tab-background-in-the-early-tabopen.patch >+.tab-background { >+ transition: visibility 1ms ease-out 20ms; >+} Are the delay and duration correct for both the opening and the closing transition? Or does it not matter because closing tabs are invisible anyway? Can you document this with a comment? >+.tabbrowser-tab[selected]:not([fadein]):not([pinned]) > .tab-stack > .tab-background { >+ visibility: hidden; >+} You should update the tab binding such that tab-background inherits all the necessary attributes and change the selector to .tab-background[selected]:not([fadein]):not([pinned]).

(In reply to Dão Gottwald [:dao] from comment #15) > >+.tab-background { > >+ transition: visibility 1ms ease-out 20ms; > >+} > > Are the delay and duration correct for both the opening and the closing > transition? Or does it not matter because closing tabs are invisible anyway? > Can you document this with a comment? What you said. Will add a comment. > >+.tabbrowser-tab[selected]:not([fadein]):not([pinned]) > .tab-stack > .tab-background { > >+ visibility: hidden; > >+} > > You should update the tab binding such that tab-background inherits all the > necessary attributes and change the selector to > .tab-background[selected]:not([fadein]):not([pinned]). Ah, thanks. That's better.

Comment on attachment 8335161 [details] [diff] [review] 0001-Bug-940262-Hide-.tab-background-in-the-early-tabopen.patch This does indeed look more smooth. This looks fine, but I'll wait for the next patch with Dao's issues fixed before giving a full-on review.

Made the suggested changes. I would have liked to do the same for the .tab-close-button because it looks a little weird that this is now also sliding from left to right but it's not as easy due to it being display:none.

Comment on attachment 8335310 [details] [diff] [review] 0001-Bug-940262-Hide-.tab-background-in-the-early-tabopen.patch >+.tab-background { >+ /** >+ * We have to explicitly specify 'visible' as the default visibility or we This really doesn't make sense to me... the initial state should already be visibility:visible. >+ * run into weird painting issues with the initial tab on startup. >+ */ >+ visibility: visible; define "weird"? I didn't notice anything over here. >+ /** >+ * This transition is only applied when opening a new tab. Closing tabs >+ * are just hidden so we don't need to adjust the offset for that. >+ */ s/offset/delay/ >+ transition: visibility 1ms ease-out 30ms; - you can use 0ms - you can omit ease-out, because it doesn't matter at all - 25ms worked fine over here (while 20ms didn't) so I'd prefer that over 30ms, assuming that the latter might be closer to hiding another frame that shouldn't be hidden

(In reply to Dão Gottwald [:dao] from comment #19) > >+.tab-background { > >+ /** > >+ * We have to explicitly specify 'visible' as the default visibility or we > > This really doesn't make sense to me... the initial state should already be > visibility:visible. Doesn't make sense to me either... > >+ * run into weird painting issues with the initial tab on startup. > >+ */ > >+ visibility: visible; > > define "weird"? I didn't notice anything over here. At least on OS X I see that if I bump the transition delay to 3000ms, on startup the .tab-background of the initial tab is hidden. After 3s it's transitioned to visible and pushes the whole tabbar down by one pixel, causing this: http://cl.ly/image/1Z2i102d0O2W Another report of a Windows user showed this: http://i.snag.gy/1vbLB.jpg The whole titlebar seems to be pushed down by some pixels here. AFAIK, switching from visibility:hidden to visible shouldn't be able to push anything anywhere but I can reliably reproduce exactly that. > >+ transition: visibility 1ms ease-out 30ms; > > - you can use 0ms > - you can omit ease-out, because it doesn't matter at all > - 25ms worked fine over here (while 20ms didn't) so I'd prefer that over > 30ms, assuming that the latter might be closer to hiding another frame that > shouldn't be hidden Ok, sure. I increased it from 20ms because of the close button.

(In reply to Tim Taubert [:ttaubert] from comment #20) > > - 25ms worked fine over here (while 20ms didn't) so I'd prefer that over > > 30ms, assuming that the latter might be closer to hiding another frame that > > shouldn't be hidden > > Ok, sure. I increased it from 20ms because of the close button. I can't say for sure that this is a reliable method but if you increase the max/min-width transition durations and the delay by 100x, 25ms seems not enough as the background is shown before the tab is wide enough and thus still makes it slide from left to right for a bit.

(In reply to Tim Taubert [:ttaubert] from comment #20) > At least on OS X I see that if I bump the transition delay to 3000ms, on > startup the .tab-background of the initial tab is hidden. After 3s it's > transitioned to visible and pushes the whole tabbar down by one pixel, > causing this: http://cl.ly/image/1Z2i102d0O2W > > Another report of a Windows user showed this: http://i.snag.gy/1vbLB.jpg The > whole titlebar seems to be pushed down by some pixels here. Would be good if we could create a reduced testcase and file a Layout bug on it. (In reply to Tim Taubert [:ttaubert] from comment #21) > I can't say for sure that this is a reliable method but if you increase the > max/min-width transition durations and the delay by 100x, 25ms seems not > enough as the background is shown before the tab is wide enough and thus > still makes it slide from left to right for a bit. Assuming 60hz and an ideal frame rate, 25ms would be roughly in the middle between two frames, so presumably a good way to ensure hitting one frame but not the subsequent one. But I could imagine the style system rounding the delay up to include the subsequent frame. Either way, you don't have to worry about this when slowing the transition down like you did, so I don't think that's a directly useful test.

(In reply to Dão Gottwald [:dao] from comment #22) > Would be good if we could create a reduced testcase and file a Layout bug on > it. Yeah, I fear that might be a difficult task. > Assuming 60hz and an ideal frame rate, 25ms would be roughly in the middle > between two frames, so presumably a good way to ensure hitting one frame but > not the subsequent one. But I could imagine the style system rounding the > delay up to include the subsequent frame. Either way, you don't have to > worry about this when slowing the transition down like you did, so I don't > think that's a directly useful test. Fair enough.

It seems that .tab-background inherits its visibility from the #TabsToolbar which is collapsed=true (visibility:collapsed) at startup. That would explain why the TabsInTitlebar calculations fail and why we're transitioning. Maximizing/restoring the window fixes the misaligned tab for me. Explicitly setting visibility:visible would thus override the inherited value and prevent the transition at startup. It would also immediately show .tab-background and thus lead to correct measurements.

Comment on attachment 8335658 [details] [diff] [review] 0001-Bug-940262-Hide-.tab-background-in-the-early-tabopen.patch >+ /** >+ * Explicitly set the visibility to override the value (collapsed) >+ * we inherit from #TabsToolbar[collapsed] at startup. >+ */ I think you actually mean the window opening rather than application startup. nit: please use this more compact comment style here: /* foo bar * foo bar. */

(In reply to Dão Gottwald [:dao] from comment #26) > >+ /** > >+ * Explicitly set the visibility to override the value (collapsed) > >+ * we inherit from #TabsToolbar[collapsed] at startup. > >+ */ > > I think you actually mean the window opening rather than application startup. Yes, thanks. That's what I wanted to say. > nit: please use this more compact comment style here: > > /* foo bar > * foo bar. */ Will do.