Closed Bug 843308 Opened 11 years ago Closed 11 years ago

Animated CSS background image doesn't restart animation when background property is cleared & then re-set

Categories

(Core :: Graphics: ImageLib, defect)

17 Branch
x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED INVALID
Tracking Status
firefox18 --- affected
firefox19 - affected
firefox20 - ---
firefox21 - ---
firefox22 - ---

People

(Reporter: u456582, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: regression)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130215130331

Steps to reproduce:

I have an SVG image with SMIL animation.
When I clic to display the image, SMIL animation starts.
When I clic to display the image a second time, SMIL animation not restarts.

Example: http://www.luigifab.info/public/svg-smil/test.html
Image: http://www.luigifab.info/public/svg-smil/error.svg.php?w=200&h=200

Tested width Firefox 19, Chrome 24, Opera 12, Safari 5, same results: not work :'(


Actual results:

The animation starts at the first display of the SVG image.


Expected results:

The animation starts at each display of the SVG image.
Component: Untriaged → SVG
Product: Firefox → Core
Regression since Firefox 17:

m-c
good=2012-08-15
bad=2012-08-16
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=86ee4deea55b&tochange=50e4ff05741e

Suspected bug:
Robert Longson — Bug 782387 - Blank SVG rendering, after clicking a link to a view fragment and then navigating back. r=dholbert
Blocks: 782387
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Version: 19 Branch → 17 Branch
As an aside: I wouldn't recommend relying on this behavior.

For images (<img>, <image>, CSS backgrounds, etc), we tend to only keep one instance of the underlying image around (which means there's only one animation timeline), and it's shared among all of the clients.  Generally, the timeline won't get restarted until we purge the image from the image-cache.  (That behavior is covered by bug 332973.  Note that at the time that bug was filed, we only supported animated GIF - not animated SVG - but it applies to SVG in image contexts as well.)

Having said that, I'm a bit surprised that Bug 782387 would have changed this behavior; I don't immediately see why.
(In reply to Daniel Holbert [:dholbert] from comment #2)
> Having said that, I'm a bit surprised that Bug 782387 would have changed
> this behavior; I don't immediately see why.

I could be wrong, just a suspected bug. ;)
No longer blocks: 782387
Ah, thanks for the clarification.

I'd say this is much more likely to have been triggered by one of khuey's image changes in that range. It sounds like we used to discard the image when it's removed from the CSS background property, and now we aren't.
Component: SVG → ImageLib
Last Good: 5709499c7c66
First Bad: 5c730c1f2274 +d5e42dcb36fd+d67c02074ced
Triggered by :
5c730c1f2274	Kyle Huey — Bug 697230: Part 1 - Centralize style image observers. r=bz
For me, I have make a simple test, just for see, I have downloaded (from /pub/mozilla.org/firefox/releases/) Firefox 4 16 17.

SVG animation restart with my example with Firefox 4/16.
It stops restarting from Firefox 17 (also tested with Nightly 22 2013-02-20).

I would have thought it was simply not implemented, but like Loic said, it's a regression.

(possible that this comments is a stupid additional comments)
It's fine now, the regression has been identified and the offending bug too. :)
Regression was first shipped in FF17, so this isn't a critical user regression. We'd accept a low risk fix though once found.
luigifab: Just to flesh out my "aside" in comment 2 a bit -- here's an experiment to illustrate that the behavior that your testcase relies on is delicate & unreliable, even in older builds, per comment 2:
 1) In an older Firefox release (where your testcase seems to work), open two copies of your testcase (in two separate tabs).
 2) Click the "Show" link in one tab.
 3) Now click the "Show" link in the other tab.
    (Hit Hide/Show/Hide/Show some more in that tab if you like)

As you'll see, we won't animate at all in the second tab -- that's because we only store one instance of the image, and it's already past that part in the animation timeline.

Your testcase happened to work in old builds, but *only* when it was the only open tab referencing the animated image, because we happened to discard images eagerly. Now we don't discard quite as eagerly -- we're keeping the image around in this case. I'm not sure if that was an intended part of khuey's patch, but I wouldn't be surprised if this change was intentional.

Either way, I'd recommend not relying on the timeline of animated CSS backgrounds -- if you want to control the timeline, just use inline SVG, and call svgNode.setCurrentTime(0) when you want it to restart.
So, fwiw, I think the difference in behavior here is because before requests were owned by the nsImageLoader and were recreated on every paint, so once we were no longer painting the background for that frame all references were gone.  Now requests are owned by the rule node tree.  This seems to imply that pieces of the rule node tree are not destroyed when the background is cleared.  I don't know if that's expected or not.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #10)
> pieces of the rule node tree are not destroyed when the background is
> cleared.  I don't know if that's expected or not.

dbaron says rule nodes are garbage-collected.  So that means that, hypothetically, if you did a bunch of style manipulation between the "hide" and "show" actions, then we'd end up garbage collecting the node that's holding onto the image, and it *would* be treated as a new image (and would start animating from the beginning) when you end up clicking "show".

So basically, this behavior changed because we switched the image-request from being owned by something very ephemeral (which drops the request immediately when the background gets cleared) to being owned by something that's lazily cleaned up, via garbage collection (so it keeps the image alive long enough to still be around when we re-instantiate the image).

So this ends up being a form of bug 332973, where a new client of an animated image is sharing state with a hidden-but-not-garbage-collected-yet client of the animated image.
 --> Marking as depending on bug 332973.
Depends on: 332973
Summary: SVG as CSS background not restart SMIL animation → Animated CSS background doesn't restart animation when background property is cleared & then re-set
Summary: Animated CSS background doesn't restart animation when background property is cleared & then re-set → Animated CSS background image doesn't restart animation when background property is cleared & then re-set
There's something else going on here, because forcing a rule tree GC after every style context destruction doesn't fix this problem.
Which is just because of the image cache.
I think we should probably mark this bug invalid.

You can force restart of the animation by using a new URI for each image, e.g. by appending HTTP query parameters to it.
@dholbert: ok

Invalid? :'(

I can reload the animation with a query string (?1234), but:
- my image is called in a CSS file (background-image), so it's difficult to do this,
- my image will be reloaded from server every-time, not one time.
As I noted at the end of comment 9 -- if you want control over the animation timeline, I'd suggest that you use inline SVG, directly in the HTML, using SVG's animation DOM APIs like "setCurrentTime" and "pauseAnimations".

Also, note that a google search for "animated css background restart" turns up a stackoverflow page with some less elegant jquery-dependent suggestions. (including one that uses query parameters like roc suggested)

Resolving as invalid, since roc, khuey, and I all agree that that's the correct resolution.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.