177661 - 2 frame GIFs do not animate

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Edwin Mons]

Some animated GIFs don't animate.  The UF comic in the URL should change after
30 seconds, but it doesn't.  When I reload the page (without refreshing) the
image changes.  Again: some GIFs animate, some don't.  Can't really put my
finger on it what causes it.

The image located at the URL works with IE6, Mozilla 1.2b and Konquerer.

Comment 1

[Edwin Mons]

Just tested the same picture in Phoenix 0.4 for Linux, and it has the same
misbehaviour there.

Comment 2

[David Tenser [:djst]]

This isn't Phoenix specific. It's the same behavior in the latest Mozilla nightly.

Comment 3

[Frank Wein [:mcsmurf]]

In Bug 119597 you should find a bug to dupe this bug to.

Comment 4

[ArronM (:paper)]

Attachment: Animate Images with 2 frames

AH! Regression from Bug 110076.
Need this in ASAP!

StartAnimation checked to see if there are more than 1 frame before starting
animation.  AppendElement moved to before StartAnimation call so that
StartAnimation sees more than one frame.

Comment 5

[ArronM (:paper)]

Expanding this bug to all 2 framed gifs.

CCing people to r= and sr= asap.  Very important to get this into 1.2!

Comment 6

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

+  // Put the following line in place when Bug 130544 gets fixed

why not just put these lines into a patch in that bug.

have no time for a real review right now. will do that in an hour or so.

Comment 7

[ArronM (:paper)]

Attachment: Animate 2 Framed GIFs

Removed comments as per Comment #6

Still forcing return of NS_OK, instead of what AppendElement returns (which is
currenty not a nsresult)

Comment 8

[ArronM (:paper)]

Comment on attachment 104788 [details] [diff] [review]
Animate 2 Framed GIFs

sorry everyone, this patch isn't correct.

For now, the best option is to back out Bug 110076.  Now is not a good time to
introduce new code ;)

Comment 9

[ArronM (:paper)]

Attachment: Animate 2 Framed GIFs

(Looks like I had more time than I thought)

- Chunk #2. Moved AppendElement to before StartAnimation() call because
StartAnimation checks if mFrames.count > 1 before starting animation.

- After StartAnimation checks the count, it gets the current frame's timeout. 
The current frame is mCompositingFrame, not mFrames(0), and
mCompositingFrame.timeout was 0.  This was the problem with the previous patch,
the 2nd frame would show immediately after the first.  Solution is to set
mCompositingFrame's timeout to mFrames(0).timemout (Chunk #1).

I've tested this patch on various GIFs and it appears to work.

Comment 10

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

Comment on attachment 104871 [details] [diff] [review]
Animate 2 Framed GIFs

r=biesi if you fix the parens here:
+      if NS_SUCCEEDED(firstFrame->GetTimeout(&timeout))
(as discussed on irc)

Comment 11

[ArronM (:paper)]

Attachment: Animate 2 Framed GIFs

((brackets) are good)
;)

Comment 12

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

Comment on attachment 104873 [details] [diff] [review]
Animate 2 Framed GIFs

r=biesi

Comment 13

[Christopher Blizzard (:blizzard)]

This is now on the 1.2 blocker list.  We need to get this fixed.

Comment 14

[tor]

Comment on attachment 104873 [details] [diff] [review]
Animate 2 Framed GIFs

sr=tor

Comment 15

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

fixing component, assignee & qa contact

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 104873 [details] [diff] [review]
Animate 2 Framed GIFs

a=roc+moz for trunk

Comment 17

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

checked in.