26759 2009-06-26 12:38:06 -0700 GIFImageDecoder is broken 2009-06-26 16:10:38 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) All All RESOLVED FIXED P1 Normal --- 1 brettw brettw creshal jmalonzo mculpepper mrobinson pkasting oldest_to_newest 128538 0 brettw 2009-06-26 12:38:06 -0700 The GIF image decoder in WebCore/platform/image-decoders used by Chromium (and I think some other ports now) is broken. Animated GIFs do not animate. The problem is a mismatch in the virtual function repetitionCount in ImageDecoder and the derived class, so the GIF version is not properly called. 128542 1 eric 2009-06-26 12:51:00 -0700 I'm pretty sure this is a dupe. Peter is your man. 128543 2 31944 brettw 2009-06-26 12:56:14 -0700 Created attachment 31944 Patch I didn't see any dupes. This is http://code.google.com/p/chromium/issues/detail?id=14805 in Chromium. 128545 3 31944 levin 2009-06-26 13:07:08 -0700 Comment on attachment 31944 Patch Won't this break ImageDecoderQt which does have the "const" on this member? 128562 4 pkasting 2009-06-26 14:19:41 -0700 I think the right thing here is to make repetitionCount() const everywhere and make GIFImageDecoder::m_repetitionCount mutable. 128567 5 31950 brettw 2009-06-26 14:35:23 -0700 Created attachment 31950 Patch v2 Indeed it does. 128569 6 brettw 2009-06-26 14:37:03 -0700 (In reply to comment #4) > I think the right thing here is to make repetitionCount() const everywhere and > make GIFImageDecoder::m_repetitionCount mutable. I did that first but it seemed somehow wrong to me. 128570 7 pkasting 2009-06-26 14:39:22 -0700 (In reply to comment #6) > (In reply to comment #4) > > I think the right thing here is to make repetitionCount() const everywhere and > > make GIFImageDecoder::m_repetitionCount mutable. > > I did that first but it seemed somehow wrong to me. It's not wrong. GIFImageDecoder::m_repetitionCount is a local cache, an implementation detail; and calling repetitionCount() does not trigger decoding. Please go this route instead of your patch v1/v2 route. 128579 8 jmalonzo 2009-06-26 15:15:06 -0700 *** Bug 26657 has been marked as a duplicate of this bug. *** 128580 9 31954 brettw 2009-06-26 15:24:27 -0700 Created attachment 31954 Patch v3 128581 10 pkasting 2009-06-26 15:27:15 -0700 (In reply to comment #9) > Created an attachment (id=31954) [review] > Patch v3 Thumbs up from me! 128585 11 31954 levin 2009-06-26 15:33:49 -0700 Comment on attachment 31954 Patch v3 Please add a link to the bug and the bug title in your change log. Something like: https://bugs.webkit.org/show_bug.cgi?id=26759 GIFImageDecoder is broken ... 128587 12 levin 2009-06-26 15:40:15 -0700 Thanks much Peter for looking it over! 128590 13 brettw 2009-06-26 16:10:38 -0700 Should be fixed in r45293 31944 2009-06-26 12:56:14 -0700 2009-06-26 15:24:27 -0700 Patch const.diff text/plain 1245 brettw ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg MmY1ZDc5Mi4uZmZhNzA5MCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMyBAQAorMjAwOS0wNi0yNiAgQnJldHQgV2lsc29u ICA8YnJldHR3QGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBSZW1vdmUgY29uc3QgZnJvbSBJbWFnZURlY29kZXIgdG8gbWF0Y2gg dGhlIEdJRkltYWdlRGVjb2RlcidzIG92ZXJyaWRlLgorICAgICAgICBUaGlzIG1pc21hdGNoIGNh dXNlZCBhbmltYXRlZCBHSUZzIHRvIG5vdCBhbmltYXRlLgorCisgICAgICAgICogcGxhdGZvcm0v aW1hZ2UtZGVjb2RlcnMvSW1hZ2VEZWNvZGVyLmg6CisgICAgICAgIChXZWJDb3JlOjpJbWFnZURl Y29kZXI6OnJlcGV0aXRpb25Db3VudCk6CisKIDIwMDktMDMtMjUgIERhdmlkIEh5YXR0ICA8aHlh dHRAYXBwbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEVyaWMgU2VpZGVsCmRpZmYgLS1n aXQgYS9XZWJDb3JlL3BsYXRmb3JtL2ltYWdlLWRlY29kZXJzL0ltYWdlRGVjb2Rlci5oIGIvV2Vi Q29yZS9wbGF0Zm9ybS9pbWFnZS1kZWNvZGVycy9JbWFnZURlY29kZXIuaAppbmRleCAxNzc1NmFj Li4wN2MzOWFlIDEwMDY0NAotLS0gYS9XZWJDb3JlL3BsYXRmb3JtL2ltYWdlLWRlY29kZXJzL0lt YWdlRGVjb2Rlci5oCisrKyBiL1dlYkNvcmUvcGxhdGZvcm0vaW1hZ2UtZGVjb2RlcnMvSW1hZ2VE ZWNvZGVyLmgKQEAgLTEzNSw3ICsxMzUsNyBAQCBwdWJsaWM6CiAgICAgdmlydHVhbCBpbnQgZnJh bWVDb3VudCgpIHsgcmV0dXJuIDE7IH0KIAogICAgIC8vIFRoZSBudW1iZXIgb2YgcmVwZXRpdGlv bnMgdG8gcGVyZm9ybSBmb3IgYW4gYW5pbWF0aW9uIGxvb3AuCi0gICAgdmlydHVhbCBpbnQgcmVw ZXRpdGlvbkNvdW50KCkgY29uc3QgeyByZXR1cm4gY0FuaW1hdGlvbk5vbmU7IH0KKyAgICB2aXJ0 dWFsIGludCByZXBldGl0aW9uQ291bnQoKSB7IHJldHVybiBjQW5pbWF0aW9uTm9uZTsgfQogCiAg ICAgLy8gQ2FsbGVkIHRvIG9idGFpbiB0aGUgUkdCQTMyQnVmZmVyIGZ1bGwgb2YgZGVjb2RlZCBk YXRhIGZvciByZW5kZXJpbmcuICBUaGUKICAgICAvLyBkZWNvZGVyIHBsdWdpbiB3aWxsIGRlY29k ZSBhcyBtdWNoIG9mIHRoZSBmcmFtZSBhcyBpdCBjYW4gYmVmb3JlIGhhbmRpbmcK 31950 2009-06-26 14:35:23 -0700 2009-06-26 15:24:27 -0700 Patch v2 const.diff text/plain 2629 brettw ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg OWIwOGJkYS4uOTcwN2NjMCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNyBAQAorMjAwOS0wNi0yNiAgQnJldHQgV2lsc29u ICA8YnJldHR3QGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBSZW1vdmUgY29uc3QgZnJvbSBJbWFnZURlY29kZXIucmVwZXRpdGlv bkNvdW50IHNvIHRoYXQgaXQgd2lsbCBtYXRjaAorICAgICAgICBHSUZJbWFnZURlY29kZXIncyBv dmVycmlkZS4gVGhpcyBtaXNtYXRjaCBpcyBjYXVzaW5nIHRoZSBHSUZJbWFnZURlY29kZXIKKyAg ICAgICAgdG8gbm90IHdvcmsuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9xdC9JbWFn ZURlY29kZXJRdC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpJbWFnZURlY29kZXJRdDo6cmVwZXRp dGlvbkNvdW50KToKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9xdC9JbWFnZURlY29kZXJR dC5oOgorICAgICAgICAqIHBsYXRmb3JtL2ltYWdlLWRlY29kZXJzL0ltYWdlRGVjb2Rlci5oOgor ICAgICAgICAoV2ViQ29yZTo6SW1hZ2VEZWNvZGVyOjpyZXBldGl0aW9uQ291bnQpOgorCiAyMDA5 LTA2LTI2ICBKZXNzaWUgQmVybGluICA8amJlcmxpbkBhcHBsZS5jb20+CiAKICAgICAgICAgUmV2 aWV3ZWQgYnkgTWFyayBSb3dlLgpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGlj cy9xdC9JbWFnZURlY29kZXJRdC5jcHAgYi9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3F0L0lt YWdlRGVjb2RlclF0LmNwcAppbmRleCA5YTdmZDc1Li5hNWUzYzkyIDEwMDY0NAotLS0gYS9XZWJD b3JlL3BsYXRmb3JtL2dyYXBoaWNzL3F0L0ltYWdlRGVjb2RlclF0LmNwcAorKysgYi9XZWJDb3Jl L3BsYXRmb3JtL2dyYXBoaWNzL3F0L0ltYWdlRGVjb2RlclF0LmNwcApAQCAtMjc0LDcgKzI3NCw3 IEBAIGludCBJbWFnZURlY29kZXJRdDo6ZnJhbWVDb3VudCgpIGNvbnN0CiAgICAgcmV0dXJuIG1f aW1hZ2VMaXN0LnNpemUoKTsKIH0KIAotaW50IEltYWdlRGVjb2RlclF0OjpyZXBldGl0aW9uQ291 bnQoKSBjb25zdAoraW50IEltYWdlRGVjb2RlclF0OjpyZXBldGl0aW9uQ291bnQoKQogewogICAg IGlmIChkZWJ1Z0ltYWdlRGVjb2RlclF0KQogICAgICAgICBxRGVidWcoKSA8PCAiIEltYWdlRGVj b2RlclF0OjpyZXBldGl0aW9uQ291bnQoKSByZXR1cm5zIiA8PCBtX2xvb3BDb3VudDsKZGlmZiAt LWdpdCBhL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvcXQvSW1hZ2VEZWNvZGVyUXQuaCBiL1dl YkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvcXQvSW1hZ2VEZWNvZGVyUXQuaAppbmRleCBlY2UwZTQ4 Li4yNzFkMWU4IDEwMDY0NAotLS0gYS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3F0L0ltYWdl RGVjb2RlclF0LmgKKysrIGIvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9xdC9JbWFnZURlY29k ZXJRdC5oCkBAIC00Nyw3ICs0Nyw3IEBAIHB1YmxpYzoKICAgICB2aXJ0dWFsIHZvaWQgc2V0RGF0 YShjb25zdCBJbmNvbWluZ0RhdGEmIGRhdGEsIGJvb2wgYWxsRGF0YVJlY2VpdmVkKTsKICAgICB2 aXJ0dWFsIGJvb2wgaXNTaXplQXZhaWxhYmxlKCk7CiAgICAgdmlydHVhbCBpbnQgZnJhbWVDb3Vu dCgpIGNvbnN0OwotICAgIHZpcnR1YWwgaW50IHJlcGV0aXRpb25Db3VudCgpIGNvbnN0OworICAg IHZpcnR1YWwgaW50IHJlcGV0aXRpb25Db3VudCgpOwogICAgIHZpcnR1YWwgUkdCQTMyQnVmZmVy KiBmcmFtZUJ1ZmZlckF0SW5kZXgoc2l6ZV90IGluZGV4KTsKIAogICAgIFFQaXhtYXAqIGltYWdl QXRJbmRleChzaXplX3QgaW5kZXgpIGNvbnN0OwpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9wbGF0Zm9y bS9pbWFnZS1kZWNvZGVycy9JbWFnZURlY29kZXIuaCBiL1dlYkNvcmUvcGxhdGZvcm0vaW1hZ2Ut ZGVjb2RlcnMvSW1hZ2VEZWNvZGVyLmgKaW5kZXggOWMyYTlkNy4uMjFmMGZiNSAxMDA2NDQKLS0t IGEvV2ViQ29yZS9wbGF0Zm9ybS9pbWFnZS1kZWNvZGVycy9JbWFnZURlY29kZXIuaAorKysgYi9X ZWJDb3JlL3BsYXRmb3JtL2ltYWdlLWRlY29kZXJzL0ltYWdlRGVjb2Rlci5oCkBAIC0yMzQsNyAr MjM0LDcgQEAgbmFtZXNwYWNlIFdlYkNvcmUgewogICAgICAgICB2aXJ0dWFsIGludCBmcmFtZUNv dW50KCkgeyByZXR1cm4gMTsgfQogCiAgICAgICAgIC8vIFRoZSBudW1iZXIgb2YgcmVwZXRpdGlv bnMgdG8gcGVyZm9ybSBmb3IgYW4gYW5pbWF0aW9uIGxvb3AuCi0gICAgICAgIHZpcnR1YWwgaW50 IHJlcGV0aXRpb25Db3VudCgpIGNvbnN0IHsgcmV0dXJuIGNBbmltYXRpb25Ob25lOyB9CisgICAg ICAgIHZpcnR1YWwgaW50IHJlcGV0aXRpb25Db3VudCgpIHsgcmV0dXJuIGNBbmltYXRpb25Ob25l OyB9CiAKICAgICAgICAgLy8gQ2FsbGVkIHRvIG9idGFpbiB0aGUgUkdCQTMyQnVmZmVyIGZ1bGwg b2YgZGVjb2RlZCBkYXRhIGZvciByZW5kZXJpbmcuICBUaGUKICAgICAgICAgLy8gZGVjb2RlciBw bHVnaW4gd2lsbCBkZWNvZGUgYXMgbXVjaCBvZiB0aGUgZnJhbWUgYXMgaXQgY2FuIGJlZm9yZSBo YW5kaW5nCg== 31954 2009-06-26 15:24:27 -0700 2009-06-26 15:33:49 -0700 Patch v3 const.diff text/plain 2146 brettw ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg ZDhlNzIzOS4uYjk3ZmM1NiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAorMjAwOS0wNi0yNiAgQnJldHQgV2lsc29u ICA8YnJldHR3QGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBNYWtlIHRoZSBHSUZJbWFnZURlY29kZXIucmVwZXRpdGlvbkNvdW50 IGZ1bmN0aW9uIGNvbnN0IHRvIG1hdGNoIHRoZQorICAgICAgICBiYXNlIGNsYXNzLiBUaGUgbWlz bWF0Y2hlZCBkZWZpbml0aW9ucyB3ZXJlIGNhdXNpbmcgdGhlIGZ1bmN0aW9uIHRvIG5vdAorICAg ICAgICBnZXQgY2FsbGVkLgorCisgICAgICAgICogcGxhdGZvcm0vaW1hZ2UtZGVjb2RlcnMvZ2lm L0dJRkltYWdlRGVjb2Rlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpHSUZJbWFnZURlY29kZXI6 OnJlcGV0aXRpb25Db3VudCk6CisgICAgICAgICogcGxhdGZvcm0vaW1hZ2UtZGVjb2RlcnMvZ2lm L0dJRkltYWdlRGVjb2Rlci5oOgorCiAyMDA5LTA2LTI2ICBKZXJlbXkgTW9za292aWNoICA8amVy ZW15QGNocm9taXVtLm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBFcmljIFNlaWRlbC4KZGlm ZiAtLWdpdCBhL1dlYkNvcmUvcGxhdGZvcm0vaW1hZ2UtZGVjb2RlcnMvZ2lmL0dJRkltYWdlRGVj b2Rlci5jcHAgYi9XZWJDb3JlL3BsYXRmb3JtL2ltYWdlLWRlY29kZXJzL2dpZi9HSUZJbWFnZURl Y29kZXIuY3BwCmluZGV4IDYwYmQ2ZGQuLjFjNmI2MmQgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvcGxh dGZvcm0vaW1hZ2UtZGVjb2RlcnMvZ2lmL0dJRkltYWdlRGVjb2Rlci5jcHAKKysrIGIvV2ViQ29y ZS9wbGF0Zm9ybS9pbWFnZS1kZWNvZGVycy9naWYvR0lGSW1hZ2VEZWNvZGVyLmNwcApAQCAtMTQ1 LDcgKzE0NSw3IEBAIGludCBHSUZJbWFnZURlY29kZXI6OmZyYW1lQ291bnQoKQogfQogCiAvLyBU aGUgbnVtYmVyIG9mIHJlcGV0aXRpb25zIHRvIHBlcmZvcm0gZm9yIGFuIGFuaW1hdGlvbiBsb29w LgotaW50IEdJRkltYWdlRGVjb2Rlcjo6cmVwZXRpdGlvbkNvdW50KCkKK2ludCBHSUZJbWFnZURl Y29kZXI6OnJlcGV0aXRpb25Db3VudCgpIGNvbnN0CiB7CiAgICAgLy8gVGhpcyB2YWx1ZSBjYW4g YXJyaXZlIGF0IGFueSBwb2ludCBpbiB0aGUgaW1hZ2UgZGF0YSBzdHJlYW0uICBNb3N0IEdJRnMK ICAgICAvLyBpbiB0aGUgd2lsZCBkZWNsYXJlIGl0IG5lYXIgdGhlIGJlZ2lubmluZyBvZiB0aGUg ZmlsZSwgc28gaXQgdXN1YWxseSBpcwpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9wbGF0Zm9ybS9pbWFn ZS1kZWNvZGVycy9naWYvR0lGSW1hZ2VEZWNvZGVyLmggYi9XZWJDb3JlL3BsYXRmb3JtL2ltYWdl LWRlY29kZXJzL2dpZi9HSUZJbWFnZURlY29kZXIuaAppbmRleCA1Yzg4OTVjLi5jZDJhMGJjIDEw MDY0NAotLS0gYS9XZWJDb3JlL3BsYXRmb3JtL2ltYWdlLWRlY29kZXJzL2dpZi9HSUZJbWFnZURl Y29kZXIuaAorKysgYi9XZWJDb3JlL3BsYXRmb3JtL2ltYWdlLWRlY29kZXJzL2dpZi9HSUZJbWFn ZURlY29kZXIuaApAQCAtNTEsNyArNTEsNyBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAgICAgICAg IHZpcnR1YWwgaW50IGZyYW1lQ291bnQoKTsKIAogICAgICAgICAvLyBUaGUgbnVtYmVyIG9mIHJl cGV0aXRpb25zIHRvIHBlcmZvcm0gZm9yIGFuIGFuaW1hdGlvbiBsb29wLgotICAgICAgICB2aXJ0 dWFsIGludCByZXBldGl0aW9uQ291bnQoKTsKKyAgICAgICAgdmlydHVhbCBpbnQgcmVwZXRpdGlv bkNvdW50KCkgY29uc3Q7CiAKICAgICAgICAgdmlydHVhbCBSR0JBMzJCdWZmZXIqIGZyYW1lQnVm ZmVyQXRJbmRleChzaXplX3QgaW5kZXgpOwogCkBAIC03OSw3ICs3OSw3IEBAIG5hbWVzcGFjZSBX ZWJDb3JlIHsKIAogICAgICAgICBib29sIG1fZnJhbWVDb3VudFZhbGlkOwogICAgICAgICBib29s IG1fY3VycmVudEJ1ZmZlclNhd0FscGhhOwotICAgICAgICBpbnQgbV9yZXBldGl0aW9uQ291bnQ7 CisgICAgICAgIG11dGFibGUgaW50IG1fcmVwZXRpdGlvbkNvdW50OwogICAgICAgICBHSUZJbWFn ZURlY29kZXJQcml2YXRlKiBtX3JlYWRlcjsKICAgICB9OwogCg==