As another hedge against bad customize mode transition performance, madhava and phlsa from UX have proposed that we special-case about:customizing in tabbrowser's addTab to skip the tab opening animation.

Can we introduce an attribute on the tab, e.g. something like 'animate' which is 'true' by default?

(In reply to Mike de Boer [:mikedeboer] from comment #1) > Can we introduce an attribute on the tab, e.g. something like 'animate' > which is 'true' by default? Yeah, I think that's the route I'm going to take, inside the tabbrowser binding.

As luck would have it, we had this nice animate boolean being composed in "addTab" which I was able to piggyback onto.

Comment on attachment 8385001 [details] [diff] [review] Patch v1 Instead of this hack, please make switchToTabHavingURI, openUILinkIn and tabbrowser's loadOneTab support optionally skipping the new tab animation and pass that through to addTab, which already supports the skipAnimation flag.

(In reply to Mike Conley (:mconley) from comment #3) > Created attachment 8385001 [details] [diff] [review] > Patch v1 > > As luck would have it, we had this nice animate boolean being composed in > "addTab" which I was able to piggyback onto. I agree - that's a better approach - thanks Dao.

Comment on attachment 8385036 [details] [diff] [review] Patch v2.1 How about this?

Comment on attachment 8385036 [details] [diff] [review] Patch v2.1 > referrerURI: aReferrerURI, > charset: aCharset, > postData: aPostData, > inBackground: loadInBackground, > allowThirdPartyFixup: aAllowThirdPartyFixup, > relatedToCurrent: aRelatedToCurrent, >- disableMCB: aDisableMCB}); >+ disableMCB: aDisableMCB, >+ skipAnimation: aSkipAnimation}); nit: move this one line up such that you don't need to touch the other line For clarity, since openUILinkIn isn't about tabs exclusively, I think the flag should be called something like skipTabOpeningAnimation (preferably briefer, but in the end clarity trumps brevity) there and then openUILinkIn should pass that as skipAnimation to loadOneTab.

Comment on attachment 8385327 [details] [diff] [review] Patch v2.2 Thanks for the feedback, Dao! What do you think of this?

Comment on attachment 8385327 [details] [diff] [review] Patch v2.2 > if (isBrowserWindow && isTabEmpty(gBrowser.selectedTab)) > gBrowser.selectedBrowser.loadURI(aURI.spec); > else >- openUILinkIn(aURI.spec, "tab"); >+ openUILinkIn(aURI.spec, "tab", aOpenParams); We should honor aOpenParams in both cases. Please replace the loadURI call with openUILinkIn(aURI.spec, "current", aOpenParams). > browser.loadOneTab(url, { > referrerURI: aReferrerURI, > charset: aCharset, > postData: aPostData, > inBackground: loadInBackground, > allowThirdPartyFixup: aAllowThirdPartyFixup, > relatedToCurrent: aRelatedToCurrent, >- disableMCB: aDisableMCB}); >+ disableMCB: aDisableMCB, >+ skipAnimation: aSkipTabAnimation}); Same comment that I made before about moving this up to leave the other line alone.

Done! Thanks Dao!

remote: https://hg.mozilla.org/integration/fx-team/rev/d67d16611935

Comment on attachment 8385480 [details] [diff] [review] Patch v2.3 (r+'d by dao) [Approval Request Comment] Bug caused by (feature/regressing bug #): Australis. User impact if declined: A slightly jankier customize mode transition. Testing completed (on m-c, etc.): Lots of local testing and a few nights on m-c. Risk to taking this patch (and alternatives if risky): The only risk is that is seems this patch causes a ~2%-~6% CART regression on OS X (see bug 980032). From my talks with UX, they're not too concerned about it, and the decrease in "motion" in the tab strip is worth the hit. Investigation will take place in bug 980032 to see what can be done about the regression. String or IDL/UUID changes made by this patch: None.

Comment on attachment 8385480 [details] [diff] [review] Patch v2.3 (r+'d by dao) [Approval Request Comment] Bug caused by (feature/regressing bug #): Australis. User impact if declined: A slightly jankier customize mode transition. Testing completed (on m-c, etc.): Lots of local testing and a few nights on m-c. Risk to taking this patch (and alternatives if risky): Very low. This will probably result in a CART regression on OS X, but this was investigated and WONTFIXED as bug 980032. String or IDL/UUID changes made by this patch: None.

Verified fixed on Windows 7 64bit, Ubuntu 12.04 and Mac OS X 10.9 using latest Aurora (build ID:20140410004003) and on Firefox 29 beta 6 (build ID: 20140407135746).