thanks to fauxpas@cc.gatech.edu for (http://slashdot.org/article.pl?sid=99/10/31/1337238) for this suggestion.

adding fauxpas to the cc list.

This would really annoy advertisers - the first frame of an animated ad banner often doesn't even identify the advertiser, let alone give a reason to click (even in theory). That's not why I'd like to see this feature, though. I'd like to see this for another reason. On sites like slashdot, cnn, infoworld, etc, the ad banner can be the last thing to finish for a modem-line user. Myself, I hit the STOP button if it looks like the page is all there (if I haven't already clicked on a link), but I have seen many, many people wait until the STOP button greys out before even scrolling. If the TCP/IP connections for gifs were dropped as soon as the first frame was displayed, the WWW wouldn't appear quite so World-wide-wait-ish to those users. BTW, the connection *should* be dropped if no animation is to be displayed: there is no reason to fetch the rest of the frames in the animated gif if they aren't going to be displayed. I can see how this could complicate implementation, though. A refinement might be to just make the webserver(s) sending the animated gifs wait to send the rest of the packets until everything else on the page is finished (i.e., until the STOP button greys) - but this would be a feature on a feature and would need a distinct pref ("Show animated gifs after page finishes loading" in addition to "Show animated gifs" and "Don't show animated gifs"). Another related preference may not scare marketers as much: "Display animated gifs once only" - done right, this would leave the gif on the last frame. This should be split off if anybody wants to work on it. BTW, fauxpas does not appear to be on the cc list at present.

Maybe fauxpas removed him-/her-self. Anyway, IE offers the option to disable animations, so advertisers should be grinning and bearing it already. And in Moz 4.x, the Stop button stays lit not only while the GIF is loading, but for the whole of the first animation loop of the graphic. This is overloading the Stop button (stop animations/stop loading). Perhaps the Stop button needs a drop-down menu, like the Back button does ...

when we do get around to implementing this, we'll have to make sure that animated gifs that are chrome:// urls still animate. otherwise, the throbber will stop working.

spam: added self to cc list as this might affect my realm --if it's implemented.

Putting in sariuh as QA Contact.

16995 may be a duplicate

Assigning all open "nobody@mozilla.org" bugs to "leger@netscape.com" to weed thru.

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

moving over to imagelib, as this is not general browser.

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

reassigning to Imagelib owner, as leger has nothing to do with this. In bug Bug 16995, there is a request to be able to right-mouse click on a image,and select a "turn animation off" option.

Technically, yes, she does. Do you see the keyword "helpwanted"? That means that this isn't an issue given to an engineer, but is a general "Help Wanted" enhancement request for anyone on the net who wishes to pick up, because nobody on Netscape's payroll has elected to implement it. Reassigning to Pref UI, anyway, since ImageLib doesn't have anything to do with this enhancement request, either.

Bulk move of all Pref UI component bugs to new Preferences component. Pref UI component will be deleted.

Backend work done with bug 33810. Removing yes/no from SUMMARY, as we also have to option to show one loop. Sean's suggestion should go into a new bug, I think. Propbaly needs more backend work. Might not even make sense, see cache. Bug 11875 is related.

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

Move to M20 target milestone.

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

Reassign since 21620 was pnunn's bug.

From 21620 (originally from 31978): "It'd be nice to have a pref that would put a bound on "infinite" so animated gifs would stop automatically after the specified time. At least on Unix, older versions of the browser used a lot of CPU cycles for animations even when the window was covered or minimized."

The attached patch implements preference-based animation of images, either full animation, one loop only, or no animation. There are three parts: preferences "advanced.animate_images" and "advanced.animate_images_once" are created in modules/libpref/init/all.js, animation options are added to the preference panel in xpfe/components/prefwindow/resources/content/pref-advanced.xul (names in pref-advanced.dtd), and lastly, an image's animate_request field of its il_container is set accordingly in modules/libimg/src/if.cpp. Admittedly, the patch is rather crude, and I see it more as a starting point for this functionality. For example, the animation mode could also be determined by setting mImageAnimationMode of nsPresContext, but I don't know the code well enough to be sure that there is always 1-1 mapping between presContext and an image. Also, I'd like hear opinions about the preference UI -- I'm not too sure whether using two checkboxes (the loop-once one indented to indicate it's a sub-option) is better than perhaps three radiobuttons, or something else entirely.

Three radio buttons, or a popup menu if pressed for space. Alternative presentations: * (Maximum space) When an image is animated, ( ) show only the first frame ( ) show the animation once (*) allow repeated looping * (Less space) Image animation: ( ) none ( ) one loop only (*) allow repeated looping * (Least space) Image animation: [allow repeated looping :^] We probably want a `Stop Animation'/`Start Animation' context menu item for animated images, too, since an animated image might be at the bottom of the page and have finished its single loop by the time I get to it. The menu item would override this preference, just for that image on that occasion.

Integrating the bound: When an image is animated, ( ) show only the first frame (*) allow repeated looping ( ) show the animation [ 1] time(s). Also, remove the multiple pref's the patch added. We only need one integer. -1 for infinate looping, 0 for no looping (first frame), 1+ for specific run count.

Tempting ... but with prefs as with variables, overloading their purpose is usually a bad idea. I might vacillate between preferring (a) 3 loops and (b) infinite loops -- switching between the two depending on my mood. If the underlying prefs were combined, the UI wouldn't remember that my favorite loop limit was 3, which would be annoying.

Right, scratch that idea. I originally thought of combining the two that were there before I saw the bound idea.

Hi all. Before we get too carried away.... the mechanism in place has 3 options: normal (whatever the animation specifies), no animation (display first frame only), and loop once. This bug is about hooking up this mechanism (associated with the nsPresContext) with a preference. -p

Davor, thanks for fixing this! Can you or Matthew create a new patch with the improved pref UI, so we can get this in, please?

I removed some glitches in the patch, e.g. the hardcoded default didn't match the one in all.js and the default pref was never queried. However, all this has no effect for me. If I go to <http://www.bucksch.org/1/projects/mozilla>, the banner loops infinitely, although there is |user_pref("advanced.animate_images", false);| in my prefs.js. Does it work for anybody?

I've appled the patch, and it doesn't work for me, either. As far as I can tell, IL_GetImage() is never getting called at all, either for chrome or content images, so checking the pref in that routine isn't helping.

I'm sorry I can't go back to check this on the machine where I developed the original patch, because it really is puzzling. I had the check for chrome URL's because they were being processed, and I was getting lockups if I tried to make them unanimated. This was on a stock RedHat Linux 6.2 to which I won't have access until September. I'll try to set up Mozilla on a Windows machine that I have during the summer and see whether I can figure out what's happening.

Not for nsbeta3

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

This looks like a partial dupe of bug #19258. Not sure which to close out as both are good, probably keep this one and leave the other for blink. Someone needs to reconcile the two. If we keep this one, be sure to take the other's keywords into this bug, in particular "access".

Would anything need to be done to the plugin architecture to ensure plugins could read this pref? I fail to see the difference to the user between aniGIFs and soundless-MPEGs, so this is likely to be something a plugin will be interested in.

I've been waiting for this long enough, and decided it was time to go in and figure out why the patch didn't work. I still don't know why IL_GetImage() is never called, but it isn't; but animation can in fact be turned off in the gif code in gif.cpp, so I've put a somewhat modified version of the patch into that file. I changed the name of the prefs to image.animate and image.animate_once, since advanced.* didn't seem very helpful, but could change them again if anyone has strong preferences. One thing about this patch: it checks the prefs every time a new gif is initialized, which is inefficient; it would probably be better to cache the pref and add an updater. I can do that any time if there's any chance of getting a review; anyone know anyone qualified to review imglib code since Pam is on sabbatical?

tor might be able to help with the review. you should keep that pref values around (as a static PRBools?) and set up a pref callback. I wouldn't check it in otherwise. adding alecf to the cc list.

where are my manners? I should have started that comment out with: "Thanks for reviving the fix."

It's likely I'm missing something obvious, but why not make PresContext a PrefListener and call nsPresContext::SetImageAnimationMode()?

I agree, save it in a member variable or something, then use prefcallbacks (or preferably, use the nsIObserver system)

reassigning to akk while pam is on sabbatical.

Now I'll really do it...

Tor: in answer to your question about using nsPresContext, it was because none of us knew about that. :-) Thanks for the suggestion! It does seem to work there, and makes more sense to put the code there; then we leave the door open for turning animations back on selectively, e.g. it might be reasonable to turn animations on when we're pointing directly at an image rather than at an html page with lots of embedded images, so that the user has a way of seeing selected animations). Stand by for another patch -- I'll move my pref updating code into nsPresContext and do the check in nsPresContext::SetShell (since that's when we actually have the scheme so we can make sure we don't turn off animations in chrome).

Here's a patch that puts the logic in the pres context. It doesn't actually cache the pref between pages, though; it allows for that (the pref value is static) but since nsPresContext doesn't cache any other prefs, but gets them again every time, I wasn't sure it was appropriate to add a mechanism to do that just for one pref. But it's easy to do at this point (though I have to store a "not set yet" value, either by inventing a bogus nsImageAnimation value or by adding an extra NotInitializedYet boolean ... Opinions?)

Could you attach a unified diff (diff -u)?

Oops! I thought I had used -u when I made that diff. Turns out my finger slipped and did -i instead. -u coming. But thinking about it last night, I decided that the half measure of using a static to hold the preference's value, but checking it at the same time as the other prefs which aren't static, didn't make sense. I'm going to get rid of the static-ness of it and treat it just like PresContext treats all its other prefs.

Tor and I both like his latest patch (01/05/01 17:45), which fixes several problems in my patch). I haven't tested the pref callback because I'm not sure how to set a pref live. Blake (or anyone else in earshot), how hard would it be to add a UI for this pref, maybe somewhere in Advanced or Debug? Adding buster, requesting sr. We want to make sure we're doing this in the right place, that SetShell when the URL is set is a reasonable place to put this code.

It's in -- woohoo! Set user_pref("image.animation_mode", "once"); or user_pref("image.animation_mode", "none"); to see it.

Filed bug 64831 to cover the UI.

akkana: if you haven't already done so, you should post a note onto the newsgroups (n.p.m.layout.checkins at least, maybe browser too) so folks know it's there and can try it out.

another thing: is there also a bug to cover the perf enhancement that we should check to see if the animation is visible in the scroll frame, and avoid decoding frames if it isn't?

Now that the feature is in, I tried it with Build 2001011008 (Linux 2.2). animation_mode "once" works like a charm, but if I set it to "none" I get frequent crashes on pages that use animated GIFs. Try setting it to "none" and visit Slashdot. It doesn't always crash - does it depend on the GIF? Anyone?

(Sorry, I tried to create an attachment, but no avail. More info on request.) As written before, force reloading a page when animation mode is "none" crashes mozilla for me after (on average) 5-7 reloads (100% reproducible). Tried with CVS snapshot from 9 a.m. GMT+1, Jan 11, 2001 on RedHat 6.2. Here's what gdb says: .... Document http://www.slashdot.org/ loaded successfully ###!!! ASSERTION: frame already has posted event: '!*FindPostedEventFor(aFrame)', file nsFrameManager.cpp, line 963 ###!!! ASSERTION: frame already has posted event: '!*FindPostedEventFor(aFrame)', file nsFrameManager.cpp, line 963 ###!!! Break: at file nsFrameManager.cpp, line 963 WARNING: not calling OnDataAvailable, file nsAsyncStreamListener.cpp, line 410 Program received signal SIGSEGV, Segmentation fault. 0x4152b213 in nsCSSFrameConstructor::CantRenderReplacedElement (this=0x85f27d0, a PresShell=0x87db068, aPresContext=0x87f4b68, aFrame=0x87522cc) at nsCSSFrameConst ructor.cpp:10248

Now that a have a recent gdb 5 snapshot, I could provide a backtrace of a crash. But is anyone reading this at all as the bug is "resolved"? Let alone interested? Should I open another bug? Hm ....

I'm interested. I'm reading. :-)

I'm reading, but I haven't been able to reproduce the crash. I've been running my dogfood optimized build with animation mode None, and haven't crashed once. If someone can get a stack trace of the crash, you bet I'll look into it, but with a crash that I can't reproduce, and without a stack trace, it's hard to do anything useful. Might be a good idea to open a new bug anyway rather than continuing the crash debugging in this bug ... assign it to me.

nsCSSFrameConstructor:10248 is: aFrame->GetParent(&parentFrame); and a simple null check wouldn't help here, because aFrame isn't null. The crash may be something subtle like imglib sets a flag to stop loading on the image, and then somewhere along the line some part of the image is deleted prematurely but there are still netlib/layout callbacks operating on it. Or something like that. Anyway, please do file a new bug (with repro instructions if you can find any, and as much in the way of stack traces as you can get), assign it to me, cc pnunn (it might have to wait until she comes back from sabbatical, since she understands imglib a lot better than I do).

I filed bug 65016 for crash.

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

Piffle! This seems to have regressed. The code hasn't been backed out, the pres context is still tracking the preference and setting mImageAnimationMode to the right thing, but it looks like the pres context's mImageAnimationMode is no longer being respected by imagelib (or someone -- I'm not sure where the regression is). Anyone hear of any recent changes that might have affected this?

Looks like this was a temporary thing. Working again on the 2/2 build. Whew!

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

Exactly how does one get this to work? Tried setting user_pref("image.animation_mode", "once"); in prefs.js and nothing happened.

I think there should be option (in rightclick popup) to load all frames of specific picture, even when in "none" animation mode.

Filed bug 69405 on the request for a more immediate UI to control animation.

This has regressed in 2001040213 (linux), perhaps due to libpr0n which is now the default. I set "image.animation_mode" to either "once" or "none", but anim gifs are not influenced in any way. http://bugzilla.mozilla.org/showattachment.cgi?attach_id=22329 can be used as a testcase. (Why am I not allowed to reopen this bug? Filing a new dup would certainly be worse, and everyone is granted that right. The mind boggles...)

user_pref("image.animation_mode", "none") doesn't work on 2001040311/Linux.

Actually, filing a new bug is more appropriate in this case. What happened is that the new imglib landed, and I suspect that turning off image animation isn't implemented yet. I'll talk to Pavlov and find out if there's already a bug tracking that issue; if not, I'll file one and help with implementing it. That makes much more sense than reopening every old bug that depends on image animation control when it's only the control mechanism that regressed. I'll leave this as REOPENED until I'm sure there's another open bug tracking the issue, though, and I'll note the new bug # here before closing again.

Filed bug 74650 for checking the pres shell's animation status in libpr0n. But I'm not entirely sure the pref is being set at the right place any more; just to be sure, I'll keep this bug open until that bug is fixed and we can verify that the pref is working again.

Bug 74169, "user pref "image.animation_mode" ignored", regression, is tracking the issue this bug was reopened for. Move discussion there?

Thanks, closing this bug again since the new one tracks the regression.

Verified

I suggest reopening this, because the option to turn off animated GIFs has been removed from the Options menu in the Firefox 3 release.