15971 2007-11-13 11:57:37 -0800 Public GIF image decoder erroneously fails if no new data has come in 2007-11-19 04:23:04 -0800 1 1 1 Unclassified WebKit Images 528+ (Nightly build) PC All RESOLVED FIXED P2 Normal --- 1 pkasting webkit-unassigned oldest_to_newest 61106 0 pkasting 2007-11-13 11:57:37 -0800 GIFImageReader.cpp (not used by Safari, but used by the Cairo/Qt ports) erroneously marks GIF decoding as having failed if we ask it to decode when no new data has come in since the last request to decode. Instead, this should just result in doing nothing. This looks like an artifact of the process of porting from the Mozilla codebase. I think in Mozilla's codebase, this function is never called unless new data has come in, whereas in WebKit, the wrapper code does not guarantee that. Therefore, I could fix this by putting a check in the wrapper that doesn't try to decode when there isn't new data. But I think making the underlying decoder fail in this case is wrong, since there's nothing dangerous about such a call (in fact, if the check in the decoder is removed entirely, we'd run all the way through the function without doing anything, and return successfully). So instead I'm planning to fix this by just changing the "return false" to "return true" in the beginning of GIFImageReader.cpp:read(). Patch coming in a bit. 61116 1 17240 pkasting 2007-11-13 14:10:08 -0800 Created attachment 17240 patch v1 Simple fix. 61170 2 alp 2007-11-13 22:17:28 -0800 I'm curious, how did you notice this? Is there a test case for this issue? A link to some online content will do.. We are getting crashes and corruption with the GIF image loader in the GTK+ port, particularly with animated GIFs, but I don't notice this patch fixing that problem. 61266 3 pkasting 2007-11-14 23:05:43 -0800 I was able to repro this pretty consistently with http://transit.511.org/providers/maps/SF_7122007102947.gif (a 3201x2840 GIF). 61439 4 17240 darin 2007-11-16 20:17:27 -0800 Comment on attachment 17240 patch v1 Looks fine, r=me. 61659 5 mrowe 2007-11-19 04:23:04 -0800 Landed in r27897. 17240 2007-11-13 14:10:08 -0800 2007-11-16 20:17:27 -0800 patch v1 patch text/plain 1325 pkasting SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAyNzc2MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTQgQEAKKzIwMDctMTEtMTMgIFBldGVyIEthc3RpbmcgIDx6ZXJvZHB4QGdtYWls LmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBo dHRwOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTk3MQorICAgICAgICBUaGUg R0lGIGRlY29kZXIgc2hvdWxkIG5vdCBmYWlsIGRlY29kaW5nIGlmIHRoZSBjYWxsZXIgYXNrcyBp dCB0bworICAgICAgICBkZWNvZGUgYWdhaW4gd2hlbiBubyBuZXcgZGF0YSBoYXMgYXJyaXZlZCBz aW5jZSB0aGUgbGFzdCBjYWxsLgorCisgICAgICAgICogcGxhdGZvcm0vaW1hZ2UtZGVjb2RlcnMv Z2lmL0dJRkltYWdlUmVhZGVyLmNwcDoKKyAgICAgICAgKEdJRkltYWdlUmVhZGVyOjpyZWFkKToK KwogMjAwNy0xMS0xMyAgRGFuIEJlcm5zdGVpbiAgPG1pdHpAYXBwbGUuY29tPgogCiAgICAgICAg IFJldmlld2VkIGJ5IEJldGggRGFraW4uCkluZGV4OiBXZWJDb3JlL3BsYXRmb3JtL2ltYWdlLWRl Y29kZXJzL2dpZi9HSUZJbWFnZVJlYWRlci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9wbGF0 Zm9ybS9pbWFnZS1kZWNvZGVycy9naWYvR0lGSW1hZ2VSZWFkZXIuY3BwCShyZXZpc2lvbiAyNzc2 MCkKKysrIFdlYkNvcmUvcGxhdGZvcm0vaW1hZ2UtZGVjb2RlcnMvZ2lmL0dJRkltYWdlUmVhZGVy LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMzc4LDggKzM3OCwxMCBAQCBpbnQgR0lGSW1hZ2VSZWFk ZXI6OmRvX2x6dyhjb25zdCB1bnNpZ25lCiBib29sIEdJRkltYWdlUmVhZGVyOjpyZWFkKGNvbnN0 IHVuc2lnbmVkIGNoYXIgKmJ1ZiwgdW5zaWduZWQgbGVuLCAKICAgICAgICAgICAgICAgICAgICAg IEdJRkltYWdlRGVjb2Rlcjo6R0lGUXVlcnkgcXVlcnksIHVuc2lnbmVkIGhhbHRBdEZyYW1lKQog ewotICBpZiAoIWxlbikKLSAgICByZXR1cm4gZmFsc2U7CisgIGlmICghbGVuKSB7CisgICAgLy8g Tm8gbmV3IGRhdGEgaGFzIGNvbWUgaW4gc2luY2UgdGhlIGxhc3QgY2FsbCwganVzdCBpZ25vcmUg dGhpcyBjYWxsLgorICAgIHJldHVybiB0cnVlOworICB9CiAKICAgY29uc3QgdW5zaWduZWQgY2hh ciAqcSA9IGJ1ZjsKIAo=