If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

user pref "image.animation_mode" ignored

RESOLVED FIXED in mozilla1.0

Status

()

Core
ImageLib
RESOLVED FIXED
17 years ago
10 years ago

People

(Reporter: Florin Iucha, Assigned: Akkana Peck)

Tracking

Trunk
mozilla1.0
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [imglib] Bug fixed, needs cleanup after old imglib gone, URL)

Attachments

(4 attachments)

(Reporter)

Description

17 years ago
I have used user_pref("image.animation_mode", "once"); since Mozilla 0.8 .

Today I have downloaded the latest Mozilla build on Windows and the animated
gifs keep cycling.

It was better yesterday when all the animated gifs were black :)
(Reporter)

Comment 1

17 years ago
user_pref("image.animation_mode", "none"); is ignored too...

Comment 2

17 years ago
Confirming on Linux RH 6.2
Mozilla CVS pull 20010403.

As you said, it worked last week. "regression" keyword is appropriate.

Even worse, you can't manually stop the animation which ESC.

(I'll attach a test case.)

Comment 3

17 years ago
Created attachment 29590 [details]
test case: animated GIF

Comment 4

17 years ago
Sorry ... above should read "... with ESC".

Florin, could you change the summary to sth. more generic, like
"User pref  image.animation_mode  ignored"?

BTW, my setup: 
"Mozilla/5.0 (X11; U; Linux 2.2.18pre11-va1.8 i686; en-US; 0.8.1)"
(Reporter)

Updated

17 years ago
Summary: user_pref("image.animation_mode", "once"); ignored in Mozilla 20010329 → user pref "image.animation_mode" ignored

Updated

17 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 5

17 years ago
It seems that the new Imglib2 code is not respecting the image.animation_mode
pref. See bug 17686, "Preference for animated image display (GIF89a, MNG)":
this regression is of the functionality added in the fix for that RFE.

Confirming, and adding comment in 17686.

Updated

17 years ago
OS: Windows NT → All
Hardware: PC → All
(Assignee)

Comment 6

17 years ago
Comments from bug 74650:
---
libpr0n doesn't check the
pres shell's GetImageAnimationMode().  

Here's how it was called in the old imglib:
http://lxr.mozilla.org/seamonkey/source/modules/libimg/src/if.cpp#1833

which was called from IL_GetImage on line 1918 of the same file.

I can add the call to libpr0n if you point me in the right direction (perhaps in
imgContainer::Init or imgContainer::StartAnimation?)
---
I've also added the test case from that bug here.
(Assignee)

Comment 7

17 years ago
*** Bug 74650 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Whiteboard: [imglib]
*** Bug 74802 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 9

17 years ago
Created attachment 30633 [details] [diff] [review]
Patch in libimg
(Assignee)

Comment 10

17 years ago
Created attachment 30634 [details] [diff] [review]
Patch to layout/base/src/nsImageFrame.cpp
(Assignee)

Updated

17 years ago
Assignee: pavlov → akkana
Whiteboard: [imglib] → [imglib] FIX IN HAND, needs r= and sr=
Target Milestone: --- → mozilla0.9
(Assignee)

Comment 11

17 years ago
Taking this bug -- I have a fix.

imgContainer doesn't have access to the pres context, so I made a new API on it
so that nsImageFrame could set the animation mode.  Then it turned out that the
GIF decoder was calling StartAnimation too early, so I moved that call into
imgContainer (it'll only be called in case of images which are actually
animated).  Finally, imgContainer had some apparently redundant code to start
the animation timers, which I replaced with a call to StartAnimation.  Animation
now works in all three modes.
(Assignee)

Comment 12

17 years ago
Pavlov, could you please review this and make sure what I've done is kosher? 
Are you the sr'er for imglib?
Status: NEW → ASSIGNED

Updated

17 years ago
Blocks: 65175

Comment 13

17 years ago
Comments: 
1. I think the animation modes should be enums in the IDL
2. why this in nsGIFDecoder2:
  -  mImageContainer->StartAnimation();  ??

Not your changes, but:

   PRBool   mCurrentFrameIsFinishedDecoding;
   PRBool   mDoneDecoding;
   PRBool   mAnimating;
+  PRUint16 mAnimationMode;

should use PRPackedBools for space reasons.
(Assignee)

Updated

17 years ago
Blocks: 64831
(Assignee)

Comment 14

17 years ago
I'm in the process of moving the enums into the idl as Simon suggested (though
unfortunately that touches a lot more files).  Stay tuned for new patch.

My patch removes StartAnimation from the nsGIFDecoder and instead handles that
within imgContainer, which seems more general and also allows us to control when
it happens (in the gif decoder it was happening too early, before any outside
class had a handle to the image and a chance to stop the animation).

The packed bool idea sounds like a good one -- I'll go ahead and make that
change in my tree, and the libpr0n reviewer (Pav?  Saari?) can veto if there's
some reason not to do that.
(Assignee)

Comment 15

17 years ago
Created attachment 30818 [details] [diff] [review]
Prospective patch for the future when libimg1 is gone
(Assignee)

Comment 16

17 years ago
Attached a patch of what had to change when I moved the enums into libpr0n. 
This doesn't currently work -- it breaks files in the old libimg, which Pav says
are going away soon.  I suggest we hold off on that part of the patch, and go
with the original simple patches for now, then do the more sweeping fix after
0.9 when Pav removes the old libimg.

Comment 17

17 years ago
r=pavlov on patches 30633 and 30634.  Please get saari to r= the imgContainer 
patch since he has done more with it than I have.

Patch 30818 looks ok to me as well, except I don't think that:
+%{C++
+typedef short nsImageAnimation;
+%}
is needed, and since it isn't used in any of the imgContainer code, i'd prefer 
it not be there.

Get saari's r= and i'll find someone to sr= it.

Comment 18

17 years ago
sr=sfraser on the 4/12 patches.
(Assignee)

Comment 19

17 years ago
Also got a verbal r=saari.  Checked in the 4/12 patches.  Leaving bug open for
the later patch, once the old imglib is excised from the build.  Resetting
target milestone and status whiteboard appropriately.
Whiteboard: [imglib] FIX IN HAND, needs r= and sr= → [imglib] Bug fixed, needs cleanup after old imglib gone
Target Milestone: mozilla0.9 → mozilla1.0

Comment 20

17 years ago
Although this is fixed (and works on all pages I've tried so far), I wonder why
the blinking eyes on resource:///res/samples/test2.html still animate. Are local
resources excluded from this pref?
(Assignee)

Comment 21

17 years ago
Yes, the code explicitly excludes resource and chrome urls, because we didn't
want this pref to turn off animation in the chrome (e.g. the throbber).

Comment 22

17 years ago
It seems that in Moz 0.9.1. (build 2001060703 WinNT) this user_pref is still
ignored. I cannot find a way to stop the animated gifs.
Moreover the ESC and context stop are not working as well.
(Assignee)

Comment 23

17 years ago
The pref is working for me.  Did someone reopen the bug, or did it not get
closed when I checked in the fix?  Mirek, what is your pref set to?

ESC / Stop not working is a different bug, assigned to Pavlov.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.