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

Animated GIF doesn't loop properly.

VERIFIED FIXED in mozilla1.9alpha8

Status

()

Core
ImageLib
--
major
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: Dolske, Assigned: Alfred Kayser)

Tracking

({regression})

Trunk
mozilla1.9alpha8
x86
All
regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
Created attachment 270246 [details]
Wheeeeeeeeeee! (loops over just a couple frames)

I noticed today that this little animation only seems to play a few frames before repeating. Looks like a regression from 196295... From IRC:

<ispiked> I confirmed it regressed between 2007-06-25-04 and 2007-06-26-04, if you want to add that to the bug.
Flags: blocking1.9?
(Reporter)

Comment 1

10 years ago
Hmm, I looked at some other animated gifs created with Adobe ImageReady, and none of them play properly.
Might be fixed by the patch in bug 386269 or the new patch in bug 196295?
(Assignee)

Comment 3

10 years ago
Created attachment 270312 [details] [diff] [review]
Patch to fix the broken 'broken gif' handling

Found it, the problem is in handling of broken gif's.

The code before:
        /* Make sure there are still rows left. If the GIF data */
        /* is corrupt, we may not get an explicit terminator.   */
        if (gs->rows_remaining == 0) {
          /* This is an illegal GIF, but we remain tolerant. */
#ifdef DONT_TOLERATE_BROKEN_GIFS
          gs->state = gif_error;
          break;
#else
          GETN(1, gif_sub_block);
#endif
        }

was with my patch changed to:
        /* Make sure there are still rows left. If the GIF data */
        /* is corrupt, we may not get an explicit terminator.   */
        if (gs->rows_remaining == 0) {
          /* This is an illegal GIF, but we remain tolerant. */
#ifdef DONT_TOLERATE_BROKEN_GIFS
          gs->state = gif_error;
#else
          GETN(1, gif_sub_block);
#endif
        }
        break;

This resulted in aborting of decoding of 'broken' gif images.
Attachment #270246 - Attachment is obsolete: true
Attachment #270312 - Flags: review?(gavin.sharp)
Comment on attachment 270312 [details] [diff] [review]
Patch to fix the broken 'broken gif' handling

You'll have to ask a peer for this module for review.
Attachment #270312 - Flags: review?(gavin.sharp)
(Assignee)

Comment 5

10 years ago
Comment on attachment 270312 [details] [diff] [review]
Patch to fix the broken 'broken gif' handling

Christian, can you review this?
Attachment #270312 - Flags: review?(cbiesinger)
May want to ask andrew (asmith, iirc) or stuart for review if biesi doesn't get to it.
Flags: blocking1.9? → blocking1.9+
(Assignee)

Updated

10 years ago
Attachment #270312 - Flags: review?(cbiesinger) → review?(pavlov)

Updated

10 years ago
Attachment #270312 - Flags: review?(pavlov) → review+
(Assignee)

Updated

10 years ago
Attachment #270312 - Flags: superreview?(tor)
Duplicate of this bug: 388174
OK -> ALL as this occurs in Windows and Linux as well.
OS: Mac OS X → All

Updated

10 years ago
Attachment #270312 - Flags: superreview?(tor) → superreview+
(Assignee)

Comment 9

10 years ago
Who can do the checkin for me?
Keywords: checkin-needed
(Assignee)

Updated

10 years ago
Blocks: 143046
Looks like there aren't many people around who can do checkins and look for / care about "checkin-needed" bugs. If it takes too long (e.g. blocks your work in other bugs), you might want to ask directly in #developers.
Target Milestone: --- → mozilla1.9 M8

Updated

10 years ago
Assignee: nobody → alfredkayser

Updated

10 years ago
Status: NEW → ASSIGNED

Comment 11

10 years ago
Checking in modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp;
/cvsroot/mozilla/modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp,v  <--  nsGIFDecoder2.cpp
new revision: 1.79; previous revision: 1.78
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
(Assignee)

Updated

10 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.