Bug 157789 - [PATCH] "Stop Animations" does not stop .gif animations
Summary: [PATCH] "Stop Animations" does not stop .gif animations
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: khtml (show other bugs)
Version: 4.0.1
Platform: Gentoo Packages Linux
: NOR major
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-02-13 18:40 UTC by Michael
Modified: 2008-09-19 03:18 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Very simple test case. (10.00 KB, application/x-tar)
2008-05-18 08:21 UTC, Adrian Kreher
Details
Patch (against 4.1.1, hopefully also applies to trunk) (6.31 KB, patch)
2008-09-19 01:48 UTC, Kevin Kofler
Details
Patch (second try) (6.63 KB, patch)
2008-09-19 02:38 UTC, Kevin Kofler
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael 2008-02-13 18:40:42 UTC
Version:            (using KDE 4.0.1)
Installed from:    Gentoo Packages
Compiler:          gcc version 4.1.2 (Gentoo 4.1.2 p1.0.2) 
OS:                Linux

Find any web page with a .gif animation.  Use any "Stop Animations" entry from any menu.  Animations proceed anyway.

Alternately, the bug reporting process should link to or query existing goals for future releases if this is a known bug that didn't make this release.
Comment 1 Adrian Kreher 2008-05-18 08:03:33 UTC
Confirmed on KDE 4.0.3 running on Fedora 9.

Visit http://www.gifs.net/subcategory/2/0/20/3D_Metal or a similar site, then right click and select "Stop Animations". Nothing happens.
Comment 2 Adrian Kreher 2008-05-18 08:21:35 UTC
Created attachment 24818 [details]
Very simple test case.
Comment 3 Kevin Kofler 2008-08-25 01:17:08 UTC
Ping? I'm bumping the severity to major because it's really really annoying to have animated GIFs blinking around when you want to read the text, IMHO this is a showstopper.
Comment 4 Kevin Kofler 2008-08-25 01:19:00 UTC
Oh and no, this is STILL not fixed in 4.1.0. :-(
Comment 5 Kevin Kofler 2008-09-19 00:08:31 UTC
I tracked this down to CachedImage::setShowAnimations in kdelibs/khtml/misc/loader.cpp being completely commented out. :-(
Comment 6 Kevin Kofler 2008-09-19 01:48:20 UTC
Created attachment 27476 [details]
Patch (against 4.1.1, hopefully also applies to trunk)

I've tested this patch to fix the issue. Any objections to me committing this to trunk and 4.1?
Comment 7 Maksim Orlovich 2008-09-19 02:06:51 UTC
Wow, thank you. Looks great.

Just one tiny nitpick: could you please remove m_showAnimations in CachedImage?

(Also, you actually don't need to loop to following frames to check for 
 animation providers --- they are always at frame 0).

... And I need to clean out all the #if 0'd QMovie mess.
Comment 8 Kevin Kofler 2008-09-19 02:13:25 UTC
Well, I copied that loop out of Image::refSize, which tries to clone an animProvider for each frame (but clone isn't actually implemented in the GifAnimProvider, so this doesn't work anyway). I'm removing my loops as they aren't actually needed, but something needs to be done to refSize.
Comment 9 Kevin Kofler 2008-09-19 02:38:56 UTC
Created attachment 27477 [details]
Patch (second try)

This addresses your nitpicks:
* no longer loops through all planes, only the "original" one
* removes the no longer used CachedImage::m_showAnimations

Tested, still works just as fine as before.
Comment 10 Maksim Orlovich 2008-09-19 03:04:54 UTC
Looks great, thank you! Please do go ahead, for both branches.
Comment 11 Kevin Kofler 2008-09-19 03:18:46 UTC
Fixed in trunk and 4.1:
http://websvn.kde.org/?view=rev&revision=862489
http://websvn.kde.org/?view=rev&revision=862491
(BUG and CCBUG are still not working, unfortunately.)