Animated images stop animating after Print / Print Preview

RESOLVED FIXED in mozilla1.9beta5

Status

()

Core
Printing: Output
RESOLVED FIXED
15 years ago
10 years ago

People

(Reporter: smontagu, Assigned: Martijn Wargers (dead))

Tracking

({regression, testcase})

Trunk
mozilla1.9beta5
regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 5 obsolete attachments)

(Reporter)

Description

15 years ago
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.

Updated

15 years ago
Blocks: 119597

Updated

15 years ago
Keywords: regression

Comment 1

12 years ago
test URL no longer exists

Comment 2

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

Updated

11 years ago
Severity: normal → minor
OS: Windows 2000 → All
Hardware: PC → All
Summary: Animated images stop animating after Print Preview → Animated images stop animating after Print / Print Preview
(Assignee)

Updated

11 years ago

Comment 3

10 years ago
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
Blocks: 182259
Severity: minor → normal
Component: Print Preview → Printing
Keywords: testcase
QA Contact: sujay → printing
(Assignee)

Comment 4

10 years ago
Created attachment 308347 [details] [diff] [review]
patch

This seems to fix it.
Attachment #308347 - Flags: review?(smontagu)
(Assignee)

Comment 5

10 years ago
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.
Attachment #308347 - Flags: review?(smontagu)
(Assignee)

Comment 6

10 years ago
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?
Attachment #308352 - Flags: review?(smontagu)
(Reporter)

Comment 7

10 years ago
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 :)
Attachment #308352 - Flags: review?(smontagu)
(Assignee)

Comment 8

10 years ago
Created attachment 308529 [details] [diff] [review]
patch

Something like this?
Attachment #308347 - Attachment is obsolete: true
Attachment #308352 - Attachment is obsolete: true
Attachment #308529 - Flags: review?(roc)
Attachment #308529 - Attachment is patch: true
Attachment #308529 - Attachment mime type: application/octet-stream → text/plain
You should share this code with nsPresContext::GetUserPreferences somehow, probably by making a method in nsPresContext that gets shared.
(Assignee)

Updated

10 years ago
Attachment #308529 - Flags: review?(roc)
(Assignee)

Comment 10

10 years ago
Created attachment 308595 [details] [diff] [review]
patch

Like this, perhaps?
(Assignee)

Updated

10 years ago
Attachment #308595 - Flags: review?(roc)
Instead of ImageAnimationModePref(), why not just add a method RestoreAnimationMode that sets the mode to the pref? Comment #6 was right :-)
(Assignee)

Comment 12

10 years ago
Created attachment 308757 [details] [diff] [review]
patch

Like this? (I keep trying)
Attachment #308529 - Attachment is obsolete: true
Attachment #308595 - Attachment is obsolete: true
Attachment #308595 - Flags: review?(roc)
(Assignee)

Updated

10 years ago
Attachment #308757 - Flags: review?(roc)
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.
(Assignee)

Comment 14

10 years ago
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)
(Assignee)

Comment 16

10 years ago
Created attachment 308775 [details] [diff] [review]
patch

Ah, yeah, doh. Sorry about all the patches.
Attachment #308757 - Attachment is obsolete: true
Attachment #308775 - Flags: review?(roc)
Attachment #308757 - Flags: review?(roc)
Attachment #308775 - Flags: superreview+
Attachment #308775 - Flags: review?(roc)
Attachment #308775 - Flags: review+
Assignee: smontagu → martijn.martijn
(Assignee)

Comment 17

10 years ago
Comment on attachment 308775 [details] [diff] [review]
patch

This is really safe.
Attachment #308775 - Flags: approval1.9?
Comment on attachment 308775 [details] [diff] [review]
patch

a1.9+=damons
Attachment #308775 - Flags: approval1.9? → approval1.9+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
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
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta5
(Assignee)

Comment 20

10 years ago
Sigh, from within a frame, animated images still don't restart animating after printing/print preview, I filed bug 428357 for that.
You need to log in before you can comment on or make changes to this bug.