My fix for bug 182259 seems to have been too heavy-handed: after print preview of a page with animated images, the images stop animating in the browser. Steps to reproduce: load http://www.burlco.info/moz/previmg/ or any page with animated images Print Preview - the images stop animating Change from Portrait to Landscape and check that the images are still not animated (this was bug 133808) Close Print Preview Expected results: animation should restart. Actual results: it doesn't.
test URL no longer exists
*** Bug 353468 has been marked as a duplicate of this bug. ***
1. visit http://en.wikipedia.org/wiki/Sieve_of_Eratosthenes (a gif) 2. print preview (or print) animation stops. animation begins where it left off if you do a page reload/refresh
Created attachment 308347 [details] [diff] [review] patch This seems to fix it.
Comment on attachment 308347 [details] [diff] [review] patch Ah, sorry, it only seems to fix the print preview case. It doesn't fix the print case.
Created attachment 308352 [details] [diff] [review] patch Ok, this duplicated code is probably too lame. I guess I should probably put it in a special function named ResetAnimationMode or something? Or is the whole idea wrong?
Comment on attachment 308352 [details] [diff] [review] patch (In reply to comment #6) > Ok, this duplicated code is probably too lame. I guess I should probably put it > in a special function named ResetAnimationMode or something? Yes, I think so. Having said that, I think you could find a better reviewer for this than me, like someone who's worked on printing code sometime in the last 5 years :)
Created attachment 308529 [details] [diff] [review] patch Something like this?
You should share this code with nsPresContext::GetUserPreferences somehow, probably by making a method in nsPresContext that gets shared.
Instead of ImageAnimationModePref(), why not just add a method RestoreAnimationMode that sets the mode to the pref? Comment #6 was right :-)
Created attachment 308757 [details] [diff] [review] patch Like this? (I keep trying)
No, I mean a very small improvement over your last patch; instead of creating ImageAnimationModePref() and calling SetAnimationMode(ImageAnimationModePref()) in a few places, just create a one-line function RestoreAnimationMode() which sets mAnimationMode = mAnimationModePref.
I tried that, but that doesn't work. I need to call SetAnimationMode to reanimate the images, no? Just changing mAnimationMode doesn't magically do that.
OK so have RestoreAnimationMode() call SetAnimationMode(mAnimationModePrefs)
Created attachment 308775 [details] [diff] [review] patch Ah, yeah, doh. Sorry about all the patches.
Comment on attachment 308775 [details] [diff] [review] patch This is really safe.
Comment on attachment 308775 [details] [diff] [review] patch a1.9+=damons
Checking in layout/base/nsDocumentViewer.cpp; /cvsroot/mozilla/layout/base/nsDocumentViewer.cpp,v <-- nsDocumentViewer.cpp new revision: 1.580; previous revision: 1.579 done Checking in layout/base/nsPresContext.h; /cvsroot/mozilla/layout/base/nsPresContext.h,v <-- nsPresContext.h new revision: 3.200; previous revision: 3.199 done
Sigh, from within a frame, animated images still don't restart animating after printing/print preview, I filed bug 428357 for that.