605197 - favicons should not animate if image.animation_mode=none

Status: REOPENED

(Core :: Graphics: ImageLib, defect)

Description

[Bobby Holley (:bholley)]

For some reason, favicons seem to take a different code path that isn't controlled by image.animation_mode. See the bug URL.

Comment 1

[Martin F.]

For bug 392866 I created a test page with a very visible favicon animation:  attachment 277379 [details]

Comment 2

[Boris Zbarsky [:bzbarsky]]

image.animation_mode only applies to content documents.  In particular, see the SchemeIs checks in nsPresContext::SetShell around mImageAnimationMode being set:

1028         if (!isChrome && !isRes)
1029           mImageAnimationMode = mImageAnimationModePref;
1030         else
1031           mImageAnimationMode = imgIContainer::kNormalAnimMode;

That's also, of course, what's going on with bug 392866: hitting esc stops animations in the _content_ document, but the favicon isn't being shown there.

Comment 3

[Bobby Holley (:bholley)]

(In reply to comment #2)
> image.animation_mode only applies to content documents.

Oh right - I guess we probably want to keep animating chrome, and we're hitting that old "favicons are actually chrome" problem.

This doesn't really seem imminently fixable - any fix would have to be a special case that would likely add more goop than we'd care to support for something like this. Any thoughts, joe?

Comment 4

[Joe Drew (not getting mail)]

hate hate hate.

I'm told that, despite being loaded by chrome, these images are still content images. Maybe we could special-case on the *image*'s scheme and not on the document?

Comment 5

[Bobby Holley (:bholley)]

(In reply to comment #4)
> I'm told that, despite being loaded by chrome, these images are still content
> images. Maybe we could special-case on the *image*'s scheme and not on the
> document?

That sounds reasonable, but I'll wait for to bz tell us if there are any snakes in the grass.

Comment 6

[Boris Zbarsky [:bzbarsky]]

That could be made to work, I think.  Involves rejiggering how animation control is done in general.

Comment 7

[Shlomi Fish]

I'd like to see this bug fixed after 
https://bugzilla.mozilla.org/show_bug.cgi?id=111373 was resolved as "WONTFIX". Also please make sure that pressing the escape button stops the favicon's animation of the current page.

Comment 8

[winterknight]

Having one preference to disable all animations would help me a lot, but it would only be a **** solution to the people who actually like animations in their web pages, which I suspect is most web users. Combining these two preferences into one seems like an arbitrary and unnecessary consolidation. Forcing users to choose between being able to watch their animated gif cats and protection from animated favicons seems weird. Following the directions of this bug would be an improvement, but not as good as doing it right.

To do this right, see this bug from 2001: 111373

Also, this bug, 605197, is just a duplicate of 383516 from 2007.

Comment 9

[Dan Mellem]

Favicons really shouldn't animate at all. As far as I know, an .ICO file doesn't have a provision for animation; Gecko is being nice and accepting a .GIF as an .ICO and animating it. Animated icons are .ANI files.

It's not a good fix, but you can clear browser.chrome.site_icons to just stop using favicon.ico altogether.

Comment 10

[IU]

Four tabs with this and Firefox is annoyingly laggy.

Comment 11

[crxssi]

If a user has image.animation_mode=none, they obviously do not want animation from web sites.  I find it impossible to believe any of those users would actually WANT animated icons on the UI (tabs and bookmarks). So disabling animation in the UI is a no-brainer when image.animation_mode=none 

I could see where the reverse might be true, however- where people might WANT animation on web sites but NOT want it in the UI.  So having a separate preference for it would be more useful, overall.  But I will take what I can get.

I have been annoyed by this issue for 14 years now (see bug 111373).

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 16 votes.
:aosmond, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 15

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.