In ImageConsumer::OnDataAvailable() (see http://lxr.mozilla.org/seamonkey/source/gfx/src/nsImageNetContextAsync.cpp#186 for more context), if the number of bytes available on the first call is < 4, then the code in libimg will be unable to determine the type of the image. On my Mac at home, running ISDN, this started to happen repeatedly, where only 3 bytes were initially available. ImageLib wants to see "GIF8" at least to know its dealing with a GIF image. This mechanism of requiring a "FirstWrite" call seems bogus. Instead, it should maintain its own state, buffering as many bytes as necessary until it knows what its dealing with. Only getting 3 bytes in the first packet may be rare, but it is possible, so the design is flawed. The end result is that images won't display when < 4 bytes are available on the first call to OnDataAvailable().

thanks for catching this, pb.

This bug has started cropping up pretty consistently. I've checked in a fix for it in mozilla/gfx/src/nsImageNetContextAsync.cpp v1.21, by checking to see if the first buffer is at least 4 bytes in size, and deferring reading if it isn't. This seems to work fairly well. Since this is an intermittent bug, it's hard to reproduce unless you simulate it in the debugger (just reduce max_read to 3 before the first read). This fix is a blatant hack. I'd rather see something where the image decoding mechanism buffers the data until enough is seen.

Is this the bug that caused images in mozilla.org to sometimes not appear on the mac and unix ? Note that these images have since been removed from the main mozilla.org page. The big start with a lizard in the middle.

Even the current banner graphic was not showing up consistently. Yes, this fixes that bug.

From what I've heard, the bug that affects the moz.org pages is still alive and affecting the page sporadically. Annnnd it affects more than just images. The temporary fix didn't help the moz.org page. Its still having problems. I'm sure this fix helps, but I don't think we are out of the woods.

I want to watch this one some more. and hopefully check it with necko. I'm pushing it out to m8. -pn

P: I'm not so sure that your hack isn't an ok way to fix this. It makes more sense to me to have the buffering done in the stream handling code instead of image decoding code. I'm willing to reconsider if you have a convincing argument for putting it in the image lib side. -pn

My counter-argument is that this isn't a robust way of handling this. What if some new format comes along where you need to see 13 bytes? Should the stream code have to know about this? At the very least, you should provide an API or compile time constant that tells the streaming code the minimum size header you need to be able to unambiguously determine the image's type. If it's always 4 bytes, then so be it, but not having this contract made explicit led to this intermittent problem.

Re-opening to ensure that pnunn saw beard's comments, and to determine whether the Resolved/Won't Fix resolution remains the case.

ok. I agree that the condition should be made explicit. All of the file formats that we currently decode and ones that are in the wings for future inclusion would be resolved in 4 bytes. So, as a temporary solution, its not out of line to specify 4 bytes. The next stage of imglib changes will use the mimetype from netlib, to determine the format. The imglib will verify the format. Most of this code will be modified for this change. I'll keep this bug open, but mark it for a later milestone. -pn

I'm not sure about people on modems, but I see this bug a LOT (via ISDN). I can only assume that those on modems will see this problem a lot more frequently than those on fast connections. For me, often the page on mozilla.org will be missing all of the curve gifs as well as the banner; instead I get text strings. For me, this seems like a bad bug to ship in a beta. I consider this to be bad rendering of the page.

This is a beta item... not PDT+ at this point

Let me get this straight: - patrick's 4 character fix is good enough for the image format we are sniffing for. - we need to create an api for this. Maybe it will be "How many characters we will need to determine" or "asking the image decoders in round robin on whether they can sniff". I dont think we will do any of this as this is a special sniff that we do for jpeg/gif. For arbitrary image formats, we wont sniff. We will trust the mimetype that comes in the header or the one we figured in the extension. So the best thing to do for this bug would be to say increase the size of the check from 4 to the minimum we need to "sniff" the 4 different hardcoded image formats. gif, jpeg : <= 4 art : 5 xbm : 8 I dont think we need to sniff ART. We had to put the sniffing in because nav4 supported the sniff even if the mimetype was wrong and there was no extension. We dont have precedence here for ART. And I dont think I care about xbm's that dont have the right mimetype. So leaving the fix at 4 and saying we need a sniff api across all image formats, new and comming in the future should close this bug. This would be an RFE. Ok ? And kathy, you image not showing is a different problem. I hope a we have a bug on that.

Per discussion marking this for M20 and leaving it the way it is.

Putting dogfood in the keyword field.

This aint dogfood. Jan why was this marked so ?

Setting to [dogfood-]. Adding helpwanted keyword

Future. off to gagan's team.

->pnunn

removing nonsensical helpwanted and dogfood status & keywords. -p

All pnunn bugs reassigned to Pav, who is taking over the imglib.

This code is no longer used. This bug has been fixed by the new image library. The new imagelib will try to get a content type from the image data, otherwise it will believe what the server gave it and continue on happily.

Verified, new image lib takes care of this issue