192631 - Animated images stop animating after Print / Print Preview

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[Simon Montagu :smontagu]

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.

Comment 1

[Wayne Mery (:wsmwk)]

test URL no longer exists

Comment 2

[Adam Guthrie]

*** Bug 353468 has been marked as a duplicate of this bug. ***

Comment 3

[Wayne Mery (:wsmwk)]

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

Comment 4

[Martijn Wargers (dead)]

Attachment: patch

This seems to fix it.

Comment 5

[Martijn Wargers (dead)]

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.

Comment 6

[Martijn Wargers (dead)]

Attachment: 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 7

[Simon Montagu :smontagu]

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 :)

Comment 8

[Martijn Wargers (dead)]

Attachment: patch

Something like this?

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

You should share this code with nsPresContext::GetUserPreferences somehow, probably by making a method in nsPresContext that gets shared.

Comment 10

[Martijn Wargers (dead)]

Attachment: patch

Like this, perhaps?

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Instead of ImageAnimationModePref(), why not just add a method RestoreAnimationMode that sets the mode to the pref? Comment #6 was right :-)

Comment 12

[Martijn Wargers (dead)]

Attachment: patch

Like this? (I keep trying)

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 14

[Martijn Wargers (dead)]

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.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OK so have RestoreAnimationMode() call SetAnimationMode(mAnimationModePrefs)

Comment 16

[Martijn Wargers (dead)]

Attachment: patch

Ah, yeah, doh. Sorry about all the patches.

Comment 17

[Martijn Wargers (dead)]

Comment on attachment 308775 [details] [diff] [review]
patch

This is really safe.

Comment 18

[Damon Sicore (:damons)]

Comment on attachment 308775 [details] [diff] [review]
patch

a1.9+=damons

Comment 19

[Reed Loden [:reed]]

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

Comment 20

[Martijn Wargers (dead)]

Sigh, from within a frame, animated images still don't restart animating after printing/print preview, I filed bug 428357 for that.