We should switch to using CSS animations for the loading & connecting throbbers. This should give us a perf win and remove allow us to guarantee smooth animation of the throbbers (with bug 706179 fixed).

I'd like to hold off on landing this patch until more of the OMTC and async animations work is completed (hopefully Firefox 16).

Removed some duplicate CSS.

Comment on attachment 627867 [details] [diff] [review] Patch for bug v2 Review of attachment 627867 [details] [diff] [review]: ----------------------------------------------------------------- This looks good, except I think we can make the animation code a bit simpler. See below: ::: browser/themes/gnomestripe/browser.css @@ +1559,5 @@ > .tab-throbber { > list-style-image: url("chrome://browser/skin/tabbrowser/connecting.png"); > + -moz-animation-duration: 960ms; > + -moz-animation-iteration-count: infinite; > + -moz-animation-name: connecting; I think we should set: -moz-animation-name: rotate; for both states and set: -moz-animation-direction: reverse; on the [progress] one or on a sepearate :not([progress]) rule. @@ +1575,5 @@ > + } > + > + to { > + -moz-transform: rotate(360deg); > + } Intuitively, we shouldn't end up with two separate frames for 0deg and 360deg by using this, and hopefully, we don't. I'll ask dbaron about that.

We can't use -moz-animation-direction:reverse because it hasn't been implemented in Gecko yet. See bug 655920 for more information. I changed the names of the animations to be more generic (rotate-clockwise, rotate-counterclockwise) and also set the animation-timing-function to linear so the animation speed will be consistent. After some more thought, I now think that we should land this for Firefox 15 and not wait for the async CSS animations. The current animation is already janky, and async CSS animations don't have a quick timeline for Windows support (it looks like work is only on Mac/Android at the moment -- since this is where OMTC is currently).

And now with optimized PNGs :-)

Comment on attachment 628518 [details] [diff] [review] Patch for bug v3 Review of attachment 628518 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. :) ::: browser/themes/gnomestripe/browser.css @@ +1572,5 @@ > + > +@-moz-keyframes rotate-clockwise { > + from { > + -moz-transform: rotate(0deg); > + } The `from` clause is actually optional in this case for our implementation of CSS animations.

Thanks! Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/970abbd6e5b0

This caused some regressions in a number of memory and latency benchmarks across all platforms: https://groups.google.com/d/topic/mozilla.dev.tree-management/4APYK_sA7e0/discussion

Backed out on inbound due the perf regressions. https://hg.mozilla.org/integration/mozilla-inbound/rev/5a095777e385

Comment on attachment 628518 [details] [diff] [review] Patch for bug v3 >+@-moz-keyframes rotate-clockwise { rename to throbber-connecting >+@-moz-keyframes rotate-counterclockwise { rename to throbber-loading >+ from { >+ -moz-transform: rotate(0deg); >+ } >+ >+ to { >+ -moz-transform: rotate(-360deg); >+ } make this from 360 to 0 instead I thought I made these comments on the first patch, but apparently I didn't...

(In reply to Jared Wein [:jaws] from comment #4) > After some more thought, I now think that we should land this for Firefox 15 > and not wait for the async CSS animations. The current animation is already > janky, The APNGs have a deliberately low frame rate to reduce the perf impact.

(In reply to Dão Gottwald [:dao] from comment #10) > >+@-moz-keyframes rotate-clockwise { > > rename to throbber-connecting > > >+@-moz-keyframes rotate-counterclockwise { > > rename to throbber-loading ... or the other way around ...

I think what we'll do for now is to just update the APNGs since the computational overhead of CSS animations isn't worth it.

What computational overhead are you referring to?

It seems to me that this just needs to wait for bug 706179.

(In reply to Dão Gottwald [:dao] from comment #14) > What computational overhead are you referring to? The stuff Jared and I were talking about between landing and backout. :) My concern about this patch is that image throbbers should be fairly efficient -- they get decoded once, so the only work as it plays is a timer firing on the main thread ("paint next frame, please") and a blit. [In theory I think that could even be entirely off-main-thread and even purely on the GPU, but not today.] CSS transforms, though, have to be computed each "frame". So ~60 times a second we're computing a transform of an image. Multiplied by each throbber on screen, since they'd all be rotating at different offsets (which seems undesirable to me). [HW accel for transforms will help -- I think that's not yet implemented? -- but for those without the CPU has to do all that work every frame.] Also, oddly enough, from Jared's demo the APNG was actually smoother under load than the transform was. Which was unexpected. I think we should call this WONTFIX, the jankyness we're seeing today isn't inherent to the APNG throbber. We can reconsider later if things change.

Once we enable OMTA for desktop, this will be cheap and smooth.

There's renewed interest in this bug - and the demo we just put together was pretty compelling. Try builds coming: https://tbpl.mozilla.org/?tree=Try&rev=309cefb6ca13 These try builds put perma-spinners on the tabs and force OMTC and OMTA on.

That try build looks great! I'd like to adjust the timing a little, but this is definitely a huge step forward. Spec/demo for the timing will be posted here soon.

Philipp, did you have the time to finish the spec yet?

Why do we need a spec here rather than translating the APNG to CSS? Seems like further design changes and enhancements could happen independently.

Dão is right, the technical part can be done without a spec. Nonetheless, here's a spec ;) http://phlsa.github.io/ff-loading-indicators/ The idea is that we can improve the perceived speed of the page load when we make the loading indicator dynamic. It makes sense to deal with this issue now to avoid a rewrite later on.

For the scaling, I'd use an easing function (like ease-in), feels smoother that way. As for a dynamic loading indicator, I don't feel much of a difference between 1 & 2 (apart from the dynamic indicator running irregularly sometimes, but I'm using Fx 27 Beta, maybe Nightly is better to test this with). How do you determine the run time for #1, though? That governs the timing function … do you want it get faster quickly and then run that fast for the rest of the loading time (which can be >10s in quite a few cases and regions)?

(In reply to Philipp Sackl [:phlsa] from comment #22) > The idea is that we can improve the perceived speed of the page load when we > make the loading indicator dynamic. It makes sense to deal with this issue > now to avoid a rewrite later on. Except that translating the current throbber is straightforward whereas a design change is not. Can we please discuss the questions asked in comment 23 in a separate bug and in the meantime make progress here?

Addressed feedback and rebased. Dao, I'm not sure why but the images weren't showing up when using list-style-image. They do appear with background-image. Applying a transform:rotate() to the list-style-image causes it to go blank on my Windows8 machine. This does not happen for background-image. Do you know why?

Comment on attachment 8407797 [details] [diff] [review] Patch v4 This needs to be updated for bug 994954. (Not sure if the new throbber will be compatible with this at all -- I haven't seen it in action yet.)

Darrin, IIRC you did the animation for the bookmark star bounce. Would you be able to recreate the new progress throbber with CSS animations?

Comment on attachment 8407797 [details] [diff] [review] Patch v4 Re-requesting review now that the new throbber was reverted.

Comment on attachment 8407797 [details] [diff] [review] Patch v4 >- list-style-image: url("chrome://browser/skin/tabbrowser/connecting.png"); >+ background-image: url("chrome://browser/skin/tabbrowser/connecting.png"); browser/themes/osx/browser.css is still using list-style-image for connecting@2x.png and loading@2x.png and would need to be updated, otherwise you end up with both a list-style-image and a background-image. That said, I'd prefer if we kept using list-style-image. Can you ping somebody from the platform team or file a bug on the images disappearing?

(In reply to Dão Gottwald [:dao] from comment #30) > I'd prefer if we kept using list-style-image. Can you ping somebody from the > platform team or file a bug on the images disappearing? Removing layer="true" from the image element seems to fix it.

Thanks for finding that the culprit was the layer="true"

Comment on attachment 8426349 [details] [diff] [review] Patch v5 Afaik layer="true" was added to improve performance. So please still ping sb. from the layout team and ask whether this is fine to remove and tell them that it's causing problems.

Matt, you added this attribute in bug 781053. Is it alright by you to remove this attribute now that we are switching to using a CSS animation?

Yes, absolutely.

<jaws> mattwoodrow: i forgot to mention in my needinfo that the presence of layer="true" meant that the transform animation caused the image to not be painted <jaws> is this a known issue? is it something we should get a bug on file for? <mattwoodrow> jaws: Worth filing a bug, but it’s not really high priority since layer=“true” is a weird xul hack that is barely used

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #36) > <jaws> mattwoodrow: i forgot to mention in my needinfo that the presence of > layer="true" meant that the transform animation caused the image to not be > painted > <jaws> is this a known issue? is it something we should get a bug on file > for? > <mattwoodrow> jaws: Worth filing a bug, but it’s not really high priority > since layer=“true” is a weird xul hack that is barely used Filed bug 1014230.

Jared, is your patch still Windows only or does it also affect other platforms?

(In reply to Philipp Sackl [:phlsa] from comment #38) > Jared, is your patch still Windows only or does it also affect other > platforms? It is for all platforms.

Added to Iteration 32.2

Hi Juan, can you review the bug to determine if further verification is required.

This makes the throbber really blurry :(.

Is there a bug on setting layers.offmainthreadcomposition.async-animations to true on desktop? That's needed for the CSS animation to really be smooth under load, isn't it?

https://bugzilla.mozilla.org/show_bug.cgi?id=788522 ?

Actually, it seems like bug 980770 is the better response to comment #45.

Is this why the connecting/loading spinners are circling around out-of-whack on Nightly lately?

(In reply to alex_mayorga from comment #48) > Is this why the connecting/loading spinners are circling around out-of-whack > on Nightly lately? Can you attach a screencast/video showing the issue you're seeing? Do you have any tab-related add-ons installed?

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #49) > (In reply to alex_mayorga from comment #48) > > Is this why the connecting/loading spinners are circling around out-of-whack > > on Nightly lately? > > Can you attach a screencast/video showing the issue you're seeing? Do you > have any tab-related add-ons installed? Filed bug 1016434 , with screenshot.

Please see http://screencast.com/t/fViRLzExHEh and let me know if you need further information from the system. These are the add-ons, none of them is tab-related: Adblock Plus 2.6 Diccionario de Español/México 1.1.3 Firefox OS Simulator 4.0.1 Iconic Firefox Menu 2.0 InlineDisposition 1.0.2.4 [DISABLED] Logitech SetPoint 6.5 [DISABLED] Nightly Tester Tools 3.7 Pixlr Grabber 2.1.4

Confirmed on WinXP using Nightly in Safe Mode

Hi Alexandra, will it be possible to have this bug verified by end of day this Friday? Our current Iteration ends on Monday June 9.

Verified as fixed with latest Nightly (Build ID: 20140602072051) on Windows 7 64bit, Mac OS X 10.9.2 and Ubuntu 14.04 32bit: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:32.0) Gecko/20100101 Firefox/32.0 Mozilla/5.0 (X11; Linux i686; rv:32.0) Gecko/20100101 Firefox/32.0 During the above testing, I've encountered bug 1016434 and bug 998267; Since both issues are tracked separately and none are blockers, marking as verified fixed.

This was backed out from Fx32 and Fx33 in bug 1016434.

I've filed bug 1059557 about the negative performance impact of this change.

This is being backed out in https://hg.mozilla.org/integration/fx-team/rev/4af29b25a7bf, mainly because of bug 1059557. Should re-land when bug 980770 is fixed.

Merge of backout: https://hg.mozilla.org/mozilla-central/rev/4af29b25a7bf

Has the performance impact of this been mesaured?

This screenshot is of the performance view of Process Explorer with 4 tabs open (about:home, about:newtab, about:newtab, about:newtab), idle without the patch.

This screenshot is of the performance view of Process Explorer with 4 tabs open (about:home, about:newtab, about:newtab, about:newtab), idle with the patch.

This screenshot is of the performance view of Process Explorer with 4 tabs open (about:home, about:newtab, about:newtab, about:newtab) without the patch. The last about:newtab tab then loads http://www.freep.com, http://www.detnews.com, and finally http://www.lsj.com, making sure to finish each load before starting the next website load. The spike in CPU coincides with the loading of the webpages.

This screenshot is of the performance view of Process Explorer with 4 tabs open (about:home, about:newtab, about:newtab, about:newtab) with the patch. The last about:newtab tab then loads http://www.freep.com, http://www.detnews.com, and finally http://www.lsj.com, making sure to finish each load before starting the next website load. The spike in CPU coincides with the loading of the webpages.

(In reply to Jeff Muizelaar [:jrmuizel] from comment #60) > Has the performance impact of this been mesaured? Based off of how bug 1059557 was filed, I measured the impact of this change by using Process Explorer and watching the performance measures of firefox.exe (the chrome process). I see no discernible difference between the performance output with and without the patch. The animation is noticeably smoother with the patch.

The other problem with this patch (or at least with the version that was checked in last time) is that it makes the "connecting" animation look slightly wrong, at least on macOS. The macOS connecting throbber is not completely symmetrical because it has a slight inner shadow towards the bottom, and this shadow is not supposed to rotate along with the rest of the icon. I don't have a good solution for this though, and somebody else will have to decide whether this is an acceptable trade-off to make.

In bug 1288986, Steven made a SVG throbber with CSS animations [1] to rotate it. And I think replace the PNG by SVG is better because the CSS animation code will be inside the SVG which we don't need to add the code to all the CSS using the png [2]. However, he noticed "it looks great on hi-res displays, it looks pretty blurry on normal displays." Steven, my desktop is 1080p, notebook is 1366x768 and both look great. What are the normal displays? I couldn't find a 1024x768 one in my office. [1] http://people.mozilla.org/~shorlander/mockups-interactive/spinner-test/spinner-01-svg.html [2] https://dxr.mozilla.org/mozilla-central/search?q=loading.png&redirect=false

NI for comment 68.

(In reply to Ting-Yu Chou [:ting] from comment #68) > And I think replace the PNG by SVG is better because the CSS animation > code will be inside the SVG which we don't need to add the code to all the > CSS using the png [2]. I believe the animation will only run on the compositor if the SVG itself is put directly into the XUL DOM, or at least embedded as a document, but not if it's embedded as an image.

(In reply to Markus Stange [:mstange] from comment #66) > The other problem with this patch (or at least with the version that was > checked in last time) is that it makes the "connecting" animation look > slightly wrong, at least on macOS. The macOS connecting throbber is not > completely symmetrical because it has a slight inner shadow towards the > bottom, and this shadow is not supposed to rotate along with the rest of the > icon. > I don't have a good solution for this though, and somebody else will have to > decide whether this is an acceptable trade-off to make. Yeah, this patch does have the rotating shadow. Clearing the checkin-needed until we can get a connecting throbber without this issue.

(In reply to Markus Stange [:mstange] from comment #70) > (In reply to Ting-Yu Chou [:ting] from comment #68) > > And I think replace the PNG by SVG is better because the CSS animation > > code will be inside the SVG which we don't need to add the code to all the > > CSS using the png [2]. > > I believe the animation will only run on the compositor if the SVG itself is > put directly into the XUL DOM, or at least embedded as a document, but not > if it's embedded as an image. Too bad :(, :birtles, is this the case?

(In reply to Ting-Yu Chou [:ting] from comment #72) > (In reply to Markus Stange [:mstange] from comment #70) > > (In reply to Ting-Yu Chou [:ting] from comment #68) > > > And I think replace the PNG by SVG is better because the CSS animation > > > code will be inside the SVG which we don't need to add the code to all the > > > CSS using the png [2]. > > > > I believe the animation will only run on the compositor if the SVG itself is > > put directly into the XUL DOM, or at least embedded as a document, but not > > if it's embedded as an image. > > Too bad :(, :birtles, is this the case? CSS Animations in SVG images don't work at all due to bug 1190881 (which is a really bad regression we've been shipping for a long time--I keep campaigning to get someone to look at it but with no luck yet).

Clear NI because of comment 73.

Sorry for another NI, but maybe you can help with comment 71?

Comment on attachment 8774521 [details] [diff] [review] Patch v5 (rebased) I need to retract my r+ from two years ago because loading.png is now used in more contexts, as mentioned in comment 68. Short of being able to put the CSS animation in an SVG, I suppose we could add a CSS class for this, e.g. "loading-icon" or just "throbber", and define the animation in global.css.

(In reply to Ting-Yu Chou [:ting] from comment #68) > In bug 1288986, Steven made a SVG throbber with CSS animations [1] to rotate > > However, he noticed "it looks great on hi-res displays, it looks pretty > blurry on normal displays." Probably bug 1271983.

Hey birtles - it looks like bug 1190881 got fixed about 2 months ago. Does that mean that the throbber in this patch will animate off the main thread now?

Looking at the CSS there I can't see any reason why it would not. It's worth checking though because there are some special cases where we might not run it on the compositor (e.g. layer size too large, conflicting animations of left/top etc.). If you're able to inspect the content using DevTools then you can quickly confirm if the animation is running on the compositor since there will be a lightning bolt mark next to any animations running on the compositor. For transform/opacity animations that *aren't* running on the compositor you can typically expand the animation and look at the tool tip which will normally tell you *why* it's not running on the compositor. If you can't inspect the content using DevTools, then your best bet is just to artificially stall the main thread with JS and check if it keeps animating or not.

(In reply to Mike Conley (:mconley) - (high latency on reviews and needinfos) from comment #78) > Hey birtles - it looks like bug 1190881 got fixed about 2 months ago. Does > that mean that the throbber in this patch will animate off the main thread > now? If you're asking is CSS transform in SVG image animating off the main thread with bug 1190881, the answer is no as what I've learned from :cjku.

Yeah, we don't seem to do animations inside image tags on the compositor. Test case: http://codepen.io/birtles/pen/bwXqMb If you right-click the image and choose 'View Image' then look at the Animations panel in the inspector you'll see it's running on the compositor: i.e. runs on the compositor when its loaded normally but not when loaded as an image document.

(In reply to Dão Gottwald [::dao] from comment #76) > Comment on attachment 8774521 [details] [diff] [review] > Patch v5 (rebased) > > I need to retract my r+ from two years ago because loading.png is now used > in more contexts, as mentioned in comment 68. Short of being able to put the > CSS animation in an SVG, I suppose we could add a CSS class for this, e.g. > "loading-icon" or just "throbber", and define the animation in global.css. Having re-read this bug in preparation for Photon, I've noticed that the only thing really blocking us from an r+ here isn't some low-level platform thing, but a refactor for the other users of the loading icon. Dao, would you be willing to accept a patch that adds a new image, loading-stillframe.png and loading-stillframe@2x.png so that we can get this into the tree, and then I'll file (and self-assign) a follow-up bug to add a new class which uses the CSS animation in all places where we use the loading throbber?

(In reply to Mike Conley (:mconley) (Catching up on reviews and needinfos) from comment #82) > Having re-read this bug in preparation for Photon, I've noticed that the > only thing really blocking us from an r+ here isn't some low-level platform > thing, but a refactor for the other users of the loading icon. > > Dao, would you be willing to accept a patch that adds a new image, > loading-stillframe.png and loading-stillframe@2x.png so that we can get this > into the tree, and then I'll file (and self-assign) a follow-up bug to add a > new class which uses the CSS animation in all places where we use the > loading throbber? Actually this is not needed anymore, CSS animations in embed SVG have been supported since a few releases now. So this should be as easy as a simple image swap.

I take what I said back, comment 81 suggests CSS animations in embed SVG are not optimized yet.

Right, CSS animations in SVG run on the compositor, but not when they are loaded via an <img> tag. Instead, it's better to load the SVG as a <img> tag, and then apply the transformation (rotation etc.) via CSS animation to the <img> element itself (or a containing div).

(In reply to Mike Conley (:mconley) (Catching up on reviews and needinfos) from comment #82) > (In reply to Dão Gottwald [::dao] from comment #76) > > Comment on attachment 8774521 [details] [diff] [review] > > Patch v5 (rebased) > > > > I need to retract my r+ from two years ago because loading.png is now used > > in more contexts, as mentioned in comment 68. Short of being able to put the > > CSS animation in an SVG, I suppose we could add a CSS class for this, e.g. > > "loading-icon" or just "throbber", and define the animation in global.css. > > Having re-read this bug in preparation for Photon, I've noticed that the > only thing really blocking us from an r+ here isn't some low-level platform > thing, but a refactor for the other users of the loading icon. > > Dao, would you be willing to accept a patch that adds a new image, > loading-stillframe.png and loading-stillframe@2x.png so that we can get this > into the tree, and then I'll file (and self-assign) a follow-up bug to add a > new class which uses the CSS animation in all places where we use the > loading throbber? Yes, except that we'll need proper images (comment 66) and SVG would be preferable over PNG.

Mind to rework the patch a bit?

This isn't quite ready since fill-opacity doesn't yet work in combination with context-fill. I had to use context-fill for connecting.svg since Stephen's original image wouldn't work on dark backgrounds.

Comment on attachment 8851655 [details] Bug 759252 - Use CSS animations for the loading and connecting throbbers. https://reviewboard.mozilla.org/r/123918/#review126420 ::: browser/themes/shared/tabbrowser/connecting.svg:3 (Diff revision 1) > +<?xml version="1.0" encoding="UTF-8"?> > +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"> > +<svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="16" height="16" viewBox="0 0 16 16"> Please remove the xlink DTD since it's unused.

Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/76ad1c764e5c Use CSS animations for the loading and connecting throbbers. r=jaws

To understand how a smoother animations impact users, what is the effort to have this behind a pref so this can be run as controlled experiment?

Impact users in what way? What's the hypothesis and what would you measure in such an experiment?

I just merged the patch to mozilla-central as part of a normal merge. I'm not the right person to ask.

I personally don't think it's worth the engineering time to try to instrument this with a pref, but I'm open to argument.

The SVG file probably shouldn't link to a DTD. I saw this in the output of a debug build: [Parent 23032] WARNING: Failed to open external DTD: publicId "-//W3C//DTD SVG 1.1//EN" systemId "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" base "file:///home/dbaron/builds/clean-mozilla-central/mozilla/browser/themes/shared/tabbrowser/connecting.svg" URL "resource://gre/res/dtd/svg11.dtd": file /home/dbaron/builds/clean-mozilla-central/mozilla/parser/htmlparser/nsExpatDriver.cpp, line 702 (It really doesn't need a DOCTYPE declaration at all.)

(In reply to David Baron :dbaron: ⌚️UTC-4 from comment #101) > The SVG file probably shouldn't link to a DTD. I saw this in the output of > a debug build: > > [Parent 23032] WARNING: Failed to open external DTD: publicId "-//W3C//DTD > SVG 1.1//EN" systemId "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" > base > "file:///home/dbaron/builds/clean-mozilla-central/mozilla/browser/themes/ > shared/tabbrowser/connecting.svg" URL "resource://gre/res/dtd/svg11.dtd": > file > /home/dbaron/builds/clean-mozilla-central/mozilla/parser/htmlparser/ > nsExpatDriver.cpp, line 702 > > (It really doesn't need a DOCTYPE declaration at all.) filed bug 1351986

Do we care about the power usage regression that this will cause?

(In reply to Jeff Muizelaar [:jrmuizel] from comment #103) > Do we care about the power usage regression that this will cause? Are you asking about power usage while the throbber is displayed or power usage after pages have been loaded? When the throbber is not busy (connecting or showing progress), it has display:none, and the `animation-name` CSS property is set to its initial value ("none"). Therefore, we shouldn't have a power regression when it's not visible. If you're asking about only when it is visible, it is a more nuanced question since it won't be visible for the whole time that Firefox is running. At this point, is it large enough of an issue to prioritize it?

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #104) > (In reply to Jeff Muizelaar [:jrmuizel] from comment #103) > > Do we care about the power usage regression that this will cause? > > Are you asking about power usage while the throbber is displayed or power > usage after pages have been loaded? While the throbber is displayed. > If you're asking about only when it is visible, it is a more nuanced > question since it won't be visible for the whole time that Firefox is > running. At this point, is it large enough of an issue to prioritize it? I guess it could be interesting to have telemetry of what percentage of the time is Firefox trying to paint at 60fps and see how that changes with this patch. That would help evaluate the importance.

For future reference, when converting UI images to SVG it would be preferable to have the switch to SVG as a separate changeset if possible. I know that would have involved making a non-animated version of connecting.png in this case, but in general it will help if we encounter Talos/perf regressions to more quickly figure out whether a perf issue is due to the use of SVG or not. In some cases the switch to SVG may not cause regressions, but in other cases it certainly will. In general be aware that SVG can be much more expensive than raster image formats, depending on the image.

setting gfx.font_rendering.opentype_svg.enabled false for rendering hw/software/security reasons, breaks this and turns it into black. can it be fixed?

(In reply to Sunil Kumar from comment #107) > setting gfx.font_rendering.opentype_svg.enabled false > for rendering hw/software/security reasons, > breaks this and turns it into black. > can it be fixed? Jonathan, is this a pref that we should still support? Sunil, can you explain for what security reasons you are disabling this pref?

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #108) > (In reply to Sunil Kumar from comment #107) > > setting gfx.font_rendering.opentype_svg.enabled false > > for rendering hw/software/security reasons, > > breaks this and turns it into black. > > can it be fixed? > > Jonathan, is this a pref that we should still support? > > Sunil, can you explain for what security reasons you are disabling this pref? In tor it allows fingerprinting, CVE-2016-9079(fixed), disabling the preference help in rendering fonts correctly/better(just like woff disabling helps sometimes and more security enhance) on some hardware. P.S: sorry for bad english

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #108) > (In reply to Sunil Kumar from comment #107) > > setting gfx.font_rendering.opentype_svg.enabled false > > for rendering hw/software/security reasons, > > breaks this and turns it into black. > > can it be fixed? > > Jonathan, is this a pref that we should still support? I'd be inclined to keep it around, as opentype-svg support is still a relatively little-used and therefore probably less-tested feature; though IIUC support has now been added in Edge, which may lead to wider adoption to the point where it becomes a required part of the platform. My question here, though, is why it has any relation to this bug. AFAICS, the patch here added some SVG images (not an OpenType-SVG font), so that pref shouldn't have any effect on it. If it's confirmed that this pref breaks the rendering of SVG images in general, that sounds like something that needs debugging. (I'm sure it didn't originally!)

(In reply to Jonathan Kew (:jfkthame) from comment #110) > I'd be inclined to keep it around, as opentype-svg support is still a > relatively little-used and therefore probably less-tested feature; though > IIUC support has now been added in Edge, which may lead to wider adoption to > the point where it becomes a required part of the platform. > > My question here, though, is why it has any relation to this bug. AFAICS, > the patch here added some SVG images (not an OpenType-SVG font), so that > pref shouldn't have any effect on it. If it's confirmed that this pref > breaks the rendering of SVG images in general, that sounds like something > that needs debugging. (I'm sure it didn't originally!) So tested it again on different platforms to see if works some thing else but it's this patch after the patch https://hg.mozilla.org/mozilla-central/rev/38894655c89e68bcd8f45d31a0d3005f2c2b53db breaks but before this patch works https://hg.mozilla.org/mozilla-central/rev/7a3f514cf8490d271ee373a1d2999e4ea4dee2d7

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #104) > (In reply to Jeff Muizelaar [:jrmuizel] from comment #103) > > Do we care about the power usage regression that this will cause? > > Are you asking about power usage while the throbber is displayed or power > usage after pages have been loaded? > > When the throbber is not busy (connecting or showing progress), it has > display:none, and the `animation-name` CSS property is set to its initial > value ("none"). Therefore, we shouldn't have a power regression when it's > not visible. > > If you're asking about only when it is visible, it is a more nuanced > question since it won't be visible for the whole time that Firefox is > running. At this point, is it large enough of an issue to prioritize it? FTR, I have just filed bug 1354080 about this, because I've been noticing excessive CPU usage while the throbber is running; IMO it has significantly degraded the user experience in my Nightly.

svg.disabled=true does not cause this. why? should not all svg be disabled?

'svg.disabled' only disables SVG support for websites content. The browser UI uses a lot of SVGs, so it can not be disabled globally. See https://bugzilla.mozilla.org/show_bug.cgi?id=1216893 for more.

Backed out for causing severe talos regressions (bug 1352085): https://hg.mozilla.org/integration/mozilla-inbound/rev/d0f1f01d369641c422db403eb5550b40cae59889 https://hg.mozilla.org/integration/mozilla-inbound/rev/5110f7c040365cacc750021b6b2aeeaa74728e7e It looks like this is a dead end, and now superseded by bug 1352119.