http://www.openbsd.org/art/banners/banner1.gif With webkit 1.2.7, that image blinks instead of animating. Backing out change listed at http://gitorious.org/webkitgtk/stable/commit/d842bb085aceec4fcfc392a7b76c92c258cd5151 fixes the issue.
This is extremely strange. How can a change in SMIL code possibly affect animated GIFs?
Are you able to confirm?
I see this bug too and reverting that code works for me as well.
This sounds impossible. SMIL is not involved when displaying gifs.
"Impossible" or "extremely strange" doesn't change the fact that webkit regressed with this change.
(In reply to comment #5) > "Impossible" or "extremely strange" doesn't change the fact that webkit regressed with this change. Nope, there must be another reason, either corrupted builds, whatever, there is no SMIL involved, believe me. This code path is never executed when displaying gifs. It's about animating SVG documents. What's the testcase you're using? Just viewing the gif, embedding the gif in a SVG document, or a HTML document? We need more input to judge.
in my original report, i pasted a direct link to a gif. Do you show it as blinking or animated?
Which svn revision is d842bb085aceec4fcfc392a7b76c92c258cd5151? Btw. I tried a chromium snapshot from March 3rd, but didn't see a problem with the gif. The image looks the same like in Safari 5, animated.
I can confirm that the gif just flickers on Archlinux i686 with libwekkit-1.2.7-1 and midori-0.3.2-1.
oops. I had two patches applied. It seems the following change is the culprit. Backing this one out makes animations work again. Does this one make more sense? :) http://trac.webkit.org/changeset/68446/trunk/WebCore/platform/image-deco ders/gif/GIFImageDecoder.cpp
http://trac.webkit.org/changeset/68446/trunk/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp
(In reply to comment #11) > http://trac.webkit.org/changeset/68446/trunk/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp What port is broken here? If you look at the full patch there, you'll see that it only changed behavior on Skia. AFAIK that's only used for Chromium, which displays this GIF correctly in my tests. I just checked the current ImageDecoder*.cpp files and no one but Skia ever returns false from this function, so unless you're using a Skia-based port this change can't have affected you.
Per Comment #9 From Tobias Ulmer 2011-03-07 06:46:28 PST I can confirm that the gif just flickers on Archlinux i686 with libwekkit-1.2.7-1 and midori-0.3.2-1. So that is not just chromium
(In reply to comment #13) > Per Comment #9 From Tobias Ulmer 2011-03-07 06:46:28 PST > > I can confirm that the gif just flickers on Archlinux i686 with libwekkit-1.2.7-1 and midori-0.3.2-1. > > So that is not just chromium Yes, I saw that comment. I have no idea what port that is or what code it uses. So it doesn't answer my question.
Here is a diff that causes "a1 = 1" to flood my xterm. So, it does indeed hit the code path. --- GIFImageDecoder.cpp.orig Mon Mar 7 16:59:34 2011 +++ GIFImageDecoder.cpp Mon Mar 7 16:49:46 2011 @@ -342,10 +342,11 @@ ASSERT(prevBuffer->status() == RGBA32Buffer::FrameComplete); if ((prevMethod == RGBA32Buffer::DisposeNotSpecified) || (prevMethod == RGBA32Buffer::DisposeKeep)) { - // Preserve the last frame as the starting state for this frame. - if (!buffer->copyBitmapData(*prevBuffer)); - return setFailed(); - } else { + int a1; + // Preserve the last frame as the starting state for this frame. + a1 = buffer->copyBitmapData(*prevBuffer); + printf("a1 = %d\n", a1); + } else { // We want to clear the previous frame to transparent, without // affecting pixels in the image outside of the frame. const IntRect& prevRect = prevBuffer->rect(); @@ -356,10 +357,11 @@ if (!buffer->setSize(bufferSize.width(), bufferSize.height())) return setFailed(); } else { + int a2; // Copy the whole previous buffer, then clear just its frame. - if (!buffer->copyBitmapData(*prevBuffer)); - return setFailed(); - for (int y = prevRect.y(); y < prevRect.bottom(); ++y) { + a2 = buffer->copyBitmapData(*prevBuffer); + printf("a2 = %d\n", a2); + for (int y = prevRect.y(); y < prevRect.bottom(); ++y) { for (int x = prevRect.x(); x < prevRect.right(); ++x) buffer->setRGBA(x, y, 0, 0, 0, 0); }
here is your patch to fix it. --- GIFImageDecoder.cpp.orig Mon Mar 7 16:59:34 2011 +++ GIFImageDecoder.cpp Mon Mar 7 17:14:09 2011 @@ -343,7 +343,7 @@ if ((prevMethod == RGBA32Buffer::DisposeNotSpecified) || (prevMethod == RGBA32Buffer::DisposeKeep)) { // Preserve the last frame as the starting state for this frame. - if (!buffer->copyBitmapData(*prevBuffer)); + if (!buffer->copyBitmapData(*prevBuffer)) return setFailed(); } else { // We want to clear the previous frame to transparent, without @@ -357,7 +357,7 @@ return setFailed(); } else { // Copy the whole previous buffer, then clear just its frame. - if (!buffer->copyBitmapData(*prevBuffer)); + if (!buffer->copyBitmapData(*prevBuffer)) return setFailed(); for (int y = prevRect.y(); y < prevRect.bottom(); ++y) { for (int x = prevRect.x(); x < prevRect.right(); ++x)
Your fork/branch/checkout/what-have-you is broken. You have a trailing semicolon after the calls to copyBitmapData() that does not exist in the changeset you cited or on the trunk. The "patch to fix" is a patch that makes the code look like what trunk already looks like (and always has looked like). To put this another way, whoever constructed the particular libraries in Archlinux did it wrong, and the fix should go into whatever location those are built out of.
ahh, the webkitgtk branch is where it exists.
You are right. That file seems correct in the trunk however it is busted in the tarball that is provided by http://webkitgtk.org/?page=download The contribute link on the webkitgtk to report bugs goes to bugs.webkit.org. So whomever runs the show on the webkitgtk checkout made a merging mistake (or something). How does one get fixes into webkitgtk? Is it not the same bug tracker?
Followup in bug 55937.