User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0) Gecko/20100101 Firefox/4.0 Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0) Gecko/20100101 Firefox/4.0 Starting from the first FF4b I installed this problem is present. This animation works fine on ALL other browser (including FF3) on all platform (WIN, MAC and LINUX). When you use 2 animated gif, with rollover, they animate the first time and then you only see ONE frame for all other over and out activity. Reproducible: Always Attached is a simple testcase (no javascript involved) that works in Fx3.6 and other but not in Fx4 Rollover gif should animate not only show first frame.

The first beta I installed was FF4b11. To try a rollback I downloaded some older versions. I installed 4b3 pre and all animations work fine except for the one on the right top, where there are 2 animations, one in the background and one in the foreground. Then I installed 4b6pre and animations work ONLY for the hover activity, when I move the mouse out I only see the first frame of the animated gif. Then I was not able to find other releases to install... I confirm again that "my site" works perfectly with all other browser I tested (FF3 up to the last 3.6.15 - Opera, IE 7 - IE 8 - Safari - Chrome) on different OS (WINXP - WIN 7 - VISTA - OSX - LINUX) where the browsers work.

Regression window: Works: http://hg.mozilla.org/mozilla-central/rev/75d2145d1bd3 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100907 Firefox/4.0b6pre ID:20100907194131 Fails: http://hg.mozilla.org/mozilla-central/rev/4bb022d84a31 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100907 Firefox/4.0b6pre ID:20100907182227 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=75d2145d1bd3&tochange=4bb022d84a31 Suspected bug: Bug 359608 - Animated GIFs are animated even when user navigates to another page

In local build(from m-c repository), build from fd33aa85de93 : fails build from d35c0d22fa71 : works build from 459a07b6bdee : works build from 75d2145d1bd3 : works Triggered by: fd33aa85de93 Alon Zakai — Bug 359608 - Animated GIFs are animated even when user navigates to another page.r=bholley,bz;sr=bz;a=blocker

Alon, can you take a look at this please?

I'm guessing the issue is that the nsImageLoader::OnStartContainer call to StartAnimation would restart the animation from the beginning if it had already finished. The new setup doesn't seem to have a way of restarting finite animations: it assumes all animations are infinite....

I'll take a look at this very soon.

On Chrome 10.0.648.114 beta, I don't see animations with the test page. Only a single frame changes when hovering. FF4 also shows a single frame change on hover, but the opposite of the frames Chrome shows. So I am confused. Are both browsers wrong here?

The site http://www.riflessivo.eu/intro/home_en.html (which involve preloading JS) works perfectly with chrome (last official release). The test file only show the last frame of animations that is, at least, a better behaviour compared to FF4. In this case we loose the animation but we can see the open and close actions correctly. Please see also the site that is more complete than the test html.

Sorry, didn't mean to clear the assignee.

Alon, Chrome shows the _last_ frame of the animation once the animation is one. It looks like Firefox 4 shows the _first_ frame of the animation. Arguably both browsers are "wrong" (in that Fx 3.6 and Opera show the whole animation), but Firefox 4's behavior seems _very_ wrong to me.

I believe it makes sense to reset mAnimationFinished in ResetAnimation(). After all, if we are resetting the animation, presumably we want it to start again even if it already finished. This fixes the no-JS testcase here and the website itself. I am pushing to try.

Comment on attachment 520359 [details] [diff] [review] patch Looks good on try, asking for review.

Hi, I would like to test it but I do not know how.. I'm sorry... I'm not a developer. Anyway, let me thanks Alon for solving this issue... I hope I'll see it in next RC.

Mario, there is no next rc planned; final release of Firefox 4 will be this Tuesday pending critical stop-ship bugs (which this bug is not). Assuming the patch is correct it will show up in Firefox 5 in a few months.

Comment on attachment 520359 [details] [diff] [review] patch Found a bug while writing a test.

Better patch, with test.

Comment on attachment 521036 [details] [diff] [review] patch v2 >diff --git a/modules/libpr0n/src/RasterImage.cpp b/modules/libpr0n/src/RasterImage.cpp >- if (ShouldAnimate()) >+ if (ShouldAnimate()) { > StartAnimation(); >+ // The animation may not have been running before, if mAnimationFinished >+ // was false (before we changed it to true in this function). So, mark the >+ // animation as running. >+ mAnimating = PR_TRUE; >+ } I almost feel like we should somehow use EvaluateAnimation or something like it here. Maybe we can unconditionally stop animation and then evaluate? It would help a lot if StopAnimation/StartAnimating set mAnimating.... >diff --git a/modules/libpr0n/test/reftest/gif/test_bug641198.html b/modules/libpr0n/test/reftest/gif/test_bug641198.html >+ document.getElementById("animated").setAttribute("class", "animated" + ((counter % 2)+1)); >+ setTimeout(doTimeout, counter == 3 ? 500 : 250); // Wait a bit more for the last one, to prevent oranges >+ } >+ counter++; This test is just *definitely* going to cause intermittent oranges, I know it :( Ehsan and I tried coming up with a MozAfterPaint method of testing animated images, but it can't be relied upon *either*. I think our only option for reliable animated image tests is to use imgILoader::LoadImage directly like the libpr0n unit tests do.

> > >diff --git a/modules/libpr0n/test/reftest/gif/test_bug641198.html b/modules/libpr0n/test/reftest/gif/test_bug641198.html > > >+ document.getElementById("animated").setAttribute("class", "animated" + ((counter % 2)+1)); > >+ setTimeout(doTimeout, counter == 3 ? 500 : 250); // Wait a bit more for the last one, to prevent oranges > >+ } > >+ counter++; > > This test is just *definitely* going to cause intermittent oranges, I know it > :( > > Ehsan and I tried coming up with a MozAfterPaint method of testing animated > images, but it can't be relied upon *either*. I think our only option for > reliable animated image tests is to use imgILoader::LoadImage directly like the > libpr0n unit tests do. I pushed this several times to try (I had other problems to work out), and I never saw an orange due to this. So while it is a risk for oranges, I at least think such oranges will be very rare if they do occur.

> This test is just *definitely* going to cause intermittent oranges The test also only tests that the image ends up showing the last frame; not that it animates, right? And it takes a full second to run?

Yes, it's a reftest, so only the last frame matters. It does test that it animated properly, though, at least in the sense that the last frame is shown and not the first (which is enough to catch the problem in this bug). It takes a second, yes. I can make it shorter, but am worried about higher risk of oranges. Do you think it is safe to do that?

I wonder whether it's better to just do a chrome test and directly listen for the notifications the animation sends....

We do have some browser chrome tests for animations, yeah, but they have limitations. I believe the safest thing regarding ensuring correct results is an actual reftest. But yes, this approach has definite downsides too...

What makes an actual reftest better? My suggestion was to use a chrome (not browser chrome) test which uses the privileges it has to install an animation listener and then uses those notifications to decide when to end the test. Then at test end you can screenshot and compare just like reftest does to make sure that no only did the animation happen but you ended up with the right bits.

Sounds interesting, is there an example of a similar test I can take a look at? 2nd question, should I not land this meanwhile? Or file a followup bug to replace the test with a better one?

Landing this in the meantime sounds fine. http://mxr.mozilla.org/mozilla-central/source/toolkit/content/tests/chrome/test_bug451286.xul might be ok to crib. Or the various things in docshell/test/chrome that include WindowSnapshot.js.

http://hg.mozilla.org/mozilla-central/rev/be4ecb32b875 I will file a followup bug for an improved test.

Filed bug 646987 for followup to improve the test.