130876 - some .gifs don't display

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect, P2)

Description

[Braindead]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.9) Gecko/20020311
BuildID:    20020311

Some of the .gif images dont display. The one's you can[not] see, are empty. But
this problem also occures with http://www.t4l-clan.de/gfx/edit.gif and
http://www.t4l-clan.de/gfx/del.gif, which are not empty.
With 0.9.8 everything is correct.

Reproducible: Always
Steps to Reproduce:
1.open the page


Actual Results:  The images dont display

Expected Results:  visible images

Comment 1

[Doron Rosenberg (IBM)]

confirming, seeing this on win98 as well.  Other people saw this on linux as well.

Comment 2

[Braindead]

This can also be seen on perlmonks.org (search-button and logo)

Comment 3

[Lorenzo Colitti]

Also seeing this at the following URLs (2002031104 WinME):

http://forum.ngi.it/smiles/love.gif
http://forum.ngi.it/smiles/insane.gif


Note that this is not a network problem because those images will refuse to load
from local files, too.

Reporter: as this is a regression from 0.9.8, could you add the regression keyword?

Comment 4

[Braindead]

As i fixed the images on my main site, i created a page with the old ones.
You can find it here: http://www.t4l-clan.de/bugzilla/bugzilla.html

Comment 5

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

Hm, when loading edit.gif using The Gimp, I get this message:

** WARNING **: GIF: bogus character 0x00, ignoring

(the image is displayed, though)

Comment 6

[Yakov Markovitch]

Attachment: A valid .gif that is not displayed by Mozilla

There is one of the images from Dr. Dobbs Algorithms Collection CD. This is
quite valid (at least, it is displayed by IE and Windows Imaging without any
complaint), whereas it isn't displayed by Mozilla, build 2002031104, W2K

Comment 7

[Lorenzo Colitti]

This is due to corrupt GIF images without a terminator byte. If there is no
terminator byte but the file ends at the end of a data block, the decoder
tolerates the file. If there are one or more extra characters after the end of a
block, but no terminator, the decoder interprets them as the start of a new
(invalid) block and marks the whole gif as invalid.

This behaviour seems to go against the GIF87a spec, which says that "Any
characters encountered between the end of a previous image and the image
separator character are to be ignored."

Comment 8

[Lorenzo Colitti]

Attachment: patch

This patch fixes the problem by only handing setting state gif_error if 0
images were decoded. If there is a missing terminator or an invalid character
but some images were decoded then it sets the state to gif_done.

It seems a reasonable thing to do but I'm not sure it's exactly the right way
to solve the problem. It would probably be better to continue reading the file
until a valid start character (',') is found. But this is a start.

Comment 9

[Lorenzo Colitti]

Attachment: Revised patch

Sorry, that was the wrong patch. This is the right one.

Comment 10

[Lorenzo Colitti]

Seeking review.
It would be nice to have this fixed for 1.0.

Comment 11

[Stuart Parmenter]

taking for 1.0.1.  we'll try to get in for 1.0, but it might be too late.

tor: can you sr= this?

Comment 12

[tor]

Comment on attachment 80253 [details] [diff] [review]
Revised patch

Remove the old #ifdef section and add a comment saying why you're
setting
the gif_done state.  Do that and sr=tor.


Comment 13

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

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

Comment 14

[Lorenzo Colitti]

Attachment: patch v3 addressing tor's comments

One question: I am not familiar with libpr0n's memory allocation scheme. In the
case of a corrupt GIF, the patch transitions to state gif_done before having
read all the file. Might this be a problem for memory management? (e.g. the
bytes not read might not be freed...)

Comment 15

[Lorenzo Colitti]

Changing URL: the images originally reported seem to be working now.

Comment 16

[ArronM (:paper)]

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

Comment 17

[Stuart Parmenter]

Comment on attachment 81475 [details] [diff] [review]
patch v3 addressing tor's comments

r=pavlov

tor please re-sr=

Comment 18

[tor]

Comment on attachment 81475 [details] [diff] [review]
patch v3 addressing tor's comments

sr=tor

Comment 19

[Lorenzo Colitti]

I can't check the patch in because I don't have CVS access.

Can someone check it in for me?

Comment 20

[R.K.Aa.]

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

Comment 21

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

Checking in GIF2.cpp
/cvsroot/mozilla/modules/libpr0n/decoders/gif/GIF2.cpp,v  <--  GIF2.cpp
new revision: 1.31; previous revision: 1.30
done


checked in on trunk

Comment 22

[david avery]

adding adt1.0.0 and patch keywords

Comment 23

[Wesha]

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

Comment 24

[Wesha]

Win2000, Mozilla Build ID 2002051304: bug no longer exist. Thanks. =^.^=

Comment 25

[Terri Preston]

Verified win 2k trunk build 2002051508, Mac OS X trunk build 2002051508 and
linux trunk build 2002051507

Comment 26

[chris hofmann]

Comment on attachment 81475 [details] [diff] [review]
patch v3 addressing tor's comments

a=chofmann,shaver for the 1.0 branch.  we need to get in asap if it's going to
make the 1.0 train.

Comment 27

[Lorenzo Colitti]

Has the fix actually been checked in to the branch?

It has the fixed1.0.0 keyword, but I can't see the fix in lxr.

Comment 28

[Dawn Endico]

yes. this was checked in to the branch 05/19/2002 20:11

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&branch=&branchtype=match&dir=&file=&filetype=match&who=pavlov%25netscape.com&whotype=match&sortby=Change+Size&hours=2&date=explicit&mindate=05%2F19%2F2002+00%3A00%3A00&maxdate=05%2F19%2F2002+23%3A00%3A00&cvsroot=%2Fcvsroot

Comment 29

[Lorenzo Colitti]

Verified with branch build 2002052009 Linux and branch build 2002052006 WinME.

Could someone verify this on Mac?

Comment 30

[Terri Preston]

Verified Mac OS X build 2002052108