224452 - If animations are disabled Mozilla should only download the first frame.

Status: NEW

(Core :: Layout: Images, Video, and HTML Frames, enhancement)

Description

[thesh_bugs]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5) Gecko/20031007
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5) Gecko/20031007

I think that if you have disabled animated gifs, there should be absolutely no
need to download more than the first frame. It would be nothing more than a
waste of bandwidth.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Ludovic Hirlimann [:Usul]]

How do you determine that the gif is animated before downloading it ?

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

hmm. this could maybe be implemented by calling imgIRequest::Cancel in a
to-implement nsImageFrame::OnStopFrame, if the animation mode is set to animate
only once.

Comment 3

[Gabriel Sjoberg]

Can someone confirm that we do indeed download the entire image using the latest
nightly build (perhaps by posting an ethereal dump)?

Comment 4

[Siraj Razick]

Attachment: Adds OnStopFrame method to nsImageFrame and calls imgIRequest::Cancel

This patch implements what was proposed by Christian Biesinger in comment #2.  
So this patch adds "OnStopFrame" to nsImageFrame and calls imgIRequest::Cancel. This patch does cancels the loading the gif when animation mode is "none". But I'm not 100% sure about the correctness and how this will effect other stuff in Mozilla + if have missed some thing. A review will help me to fix this bug,  So I'm marking this r?. 
Thank you :).

Comment 5

[Boris Zbarsky [:bzbarsky]]

Canceling needs an error nsresult; what you're passing in is not an nsresult at all..

Also, this would only help for <img>, not for backgrounds, border-image, etc...

This would also screw up images that are used in multiple prescontexts if animation is enabled in some but disabled in others, no?

Comment 6

[Siraj Razick]

(In reply to comment #5)
> Canceling needs an error nsresult; what you're passing in is not an nsresult at
> all..
Thank you for the review :). I'll post another w-i-p patch with that fix. 
> 
> Also, this would only help for <img>, not for backgrounds, border-image, etc...
> 
> This would also screw up images that are used in multiple prescontexts if
> animation is enabled in some but disabled in others, no?
Oops yeah, I will investigate this... I'm totally new to Mozilla code, So if you (or any one else) have any tips on how to solve this problem It would help :). I'll keep playing with it 

Thanks again!

Comment 7

[Boris Zbarsky [:bzbarsky]]

I don't really have a good solution to the sharing issue, unfortunately.  :(

Comment 8

[Siraj Razick]

(In reply to comment #7)
> I don't really have a good solution to the sharing issue, unfortunately.  :(

:-). it was very educational to see how images are handled. I'll experiment a bit more to see if I can find a solution.

Comment 9

[Siraj Razick]

Comment on attachment 345080 [details] [diff] [review]
Adds OnStopFrame method to nsImageFrame and calls imgIRequest::Cancel

Marking obsolete, since I'm going to attach another, slightly updated patch.

Comment 10

[Siraj Razick]

Attachment: W-I-P Version 2 of the patch , pass a nsresult when calling Cancel

Fixes the fist issue pointed out by Boris Zbarsky on comment #5. I'm not making for review since the other problems still remain to be solved.

Comment 11

[Siraj Razick]

Comment on attachment 345270 [details] [diff] [review]
W-I-P Version 2 of the patch , pass  a nsresult when calling Cancel 

updated patch coming up

Comment 12

[Siraj Razick]

Attachment: W-I-P Version 3, does not Call cancel on non-animating images

the previous patch was calling Cancel on all types of images, I'm using Frame Count to see if it's an animating image or not.. is there any other better way to detect animating and none animating images ?.

Comment 13

[Siraj Razick]

Attachment: W-I-P Version 3, does not Call cancel on non-animating images

oops, I just realised that I had attached the wrong file last time.

Comment 14

[Brandon Mitchell]

With image.animation_mode = none, Firefox doesn't just download the entire GIF image, but also decompresses all the frames and stores them in memory. The latter is a more serious problem than the former: for example, the GIF that prompted me to file Bug 514841 only consumes about 10 MB of bandwidth but takes up at least a gigabyte of memory when fully decompressed. That GIF has 1000 frames, so Firefox is unnecessarily storing 999 frames (that aren't going to be used because the animation has been disabled) in memory.

I would consider this to be less of a feature request (as it is currently classified) and more of a *bug* about fixing a serious flaw in the implementation of pref image.animation_mode.