Last Comment Bug 597174 - Firefox 3.6.10 doesn't render gifs correctly, w/ certain system-cairo versions. Displays (flashes) only the first frame.
: Firefox 3.6.10 doesn't render gifs correctly, w/ certain system-cairo version...
Status: RESOLVED WORKSFORME
:
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: 1.9.2 Branch
: x86 Linux
: -- normal with 4 votes (vote)
: ---
Assigned To: Rafał Mużyło
:
: Milan Sreckovic [:milan]
Mentors:
: 552986 (view as bug list)
Depends on: 534787 545513 546272
Blocks:
  Show dependency treegraph
 
Reported: 2010-09-16 13:35 PDT by Alexander
Modified: 2011-07-28 12:10 PDT (History)
30 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
(mostly a rollup of several backports) fix for animated gif problem (4.30 KB, patch)
2010-11-30 18:31 PST, Rafał Mużyło
no flags Details | Diff | Splinter Review

Description Alexander 2010-09-16 13:35:41 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.10) Gecko/20100916 Firefox/3.6.10
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.10) Gecko/20100916 Firefox/3.6.10

Firefox 3.6.10 doesn't render  gifs correctly. Displays (flashes) only the first frame. 

It seems that it works fine with big gifs. Small gifs though (like) smilies doesn't render ok.

E.g.:
http://i.imgur.com/hfZm5.gif
http://imgur.com/Znj5Z.gif
http://comments.deviantart.com/emoticons

Extra infos:
https://bbs.archlinux.org/viewtopic.php?id=105024
https://bugs.archlinux.org/task/20868


Reproducible: Always

Steps to Reproduce:
1. Try to open some of the following gifs.

http://i.imgur.com/hfZm5.gif
http://imgur.com/Znj5Z.gif
http://comments.deviantart.com/emoticons

Actual Results:  
It only shows the first frame of the gif and it flashes it fast.

Expected Results:  
Show a smooth animation of the gif.
Comment 1 jpike 2010-09-21 03:05:24 PDT
Exact same problem here.
Comment 2 jpike 2010-09-21 03:06:12 PDT
No extensions loaded. Using "nouveau" display drivers on xorg 1.9 Linux.
Comment 3 Ionut Biru 2010-09-22 16:07:15 PDT
happens when using cairo 1.10.0. with 1.8.10 is fine
Comment 4 Alexander 2010-09-23 02:53:45 PDT
I am having the issue with cairo-lcd 1.8.10
Comment 5 Alexander 2010-09-23 02:57:32 PDT
Ignore the above. It is a cairo 1.10.0 issue (I had a look to the other laptop of mine with cairo-lcd 1.8.10).
Comment 6 Thorsten Leemhuis 2010-10-05 22:58:39 PDT
Reports about this bug can be found in various distro bug trackers:

Fedora:
https://bugzilla.redhat.com/show_bug.cgi?id=628331

Gentoo:
http://bugs.gentoo.org/show_bug.cgi?id=337813

Mandriva:
https://qa.mandriva.com/show_bug.cgi?id=60738
Comment 7 Hicham 2010-10-28 09:20:07 PDT
I hope mozilla won't suggest to use the bundled cairo for gecko-1.9.x
Comment 8 Matěj Cepl 2010-11-04 15:15:10 PDT
I think this comment from Benjamin Otte (https://bugzilla.redhat.com/show_bug.cgi?id=628331#c19; a maintainer of Cairo) could be relevant:

> This is most likely related to the image loader loading data into a Cairo image
surface but forgetting to call cairo_surface_mark_dirty() after and/or
cairo_surface_flush() before the modifications. As Cairo 1.10 is much stricter
about enforcing these requirement than 1.8 was, it might indeed be that the bug
does only show up in Cairo 1.10.

> So I'm reassigning it back to Firefox.
Comment 9 Vlad Dascalu 2010-11-07 11:41:09 PST
Related / dup-candidate: bug 552986 .
Comment 10 Željko Jovanović 2010-11-19 18:42:42 PST
I've noticed this bug after switching to Cairo 1.10.0. Interestingly, it shows in Firefox 3.6.12, but Firefox 4.0b7 works fine (with the same Cairo version).
Comment 11 Chris 2010-11-21 05:05:00 PST
I use both Slackware-current and Slackware64-current. We had a major update to -current recently and I see this problem but ONLY with Slackware64 and not with the 32-bit Slackware. GIF animations are properly display in Slackware (32-bit)

Both have been upgraded to use cairo 1.10.0. After regressing to cairo 1.8.8 in Slackware64 GIF animations display correctly. 

I tested this with firefox-4.0b7 and cairo 1.10.0 with zero problems, GIF animations are properly displayed. 

I also see this behavior in Seamonkey and Thunderbird.
Comment 12 Chris 2010-11-21 05:09:10 PST
Left out the Firefox version : Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.12) Gecko/20101028 Firefox/3.6.12
Comment 13 Rafał Mużyło 2010-11-30 18:31:59 PST
Created attachment 494277 [details] [diff] [review]
(mostly a rollup of several backports) fix for animated gif problem

This is (more or less) a backport of 3 commits from http://hg.mozilla.org/releases/mozilla-2.0 at modules/libpr0n/ - I'm not sure if I got all, that's required
and (probably) commit fixing bug 546272 isn't really needed here, but it doesn't seem to hurt and animated gifs work.

Tested with xulrunner-1.9.2.12
Comment 14 Joe Drew (not getting mail) 2010-11-30 19:28:56 PST
Rafał, what are the bug numbers you backported?
Comment 15 Jory A. Pratt 2010-12-01 07:16:59 PST
(In reply to comment #13)
> Created attachment 494277 [details] [diff] [review]
> fix for animated gif problem
> 
> This is (more or less) a backport of 3 commits from
> http://hg.mozilla.org/releases/mozilla-2.0 at modules/libpr0n/ - I'm not sure
> if I got all, that's required
> and (probably) commit fixing bug 546272 isn't really needed here, but it
> doesn't seem to hurt and animated gifs work.
> 
> Tested with xulrunner-1.9.2.12

I would prefer to see bug 546272 brought along but as stated it is not needed. Joe I have tested on windows mac and linux all are fine as far as official builds distrubuted by mozilla would be. I have also tested on linux for system cairo and problem is resolved.
Comment 16 Rafał Mużyło 2010-12-01 08:00:12 PST
I may have phrased it too a bit badly.
This patch came from parts of following commits:
5aadb09fdc70c05574fb759a51d505d783962789 (modules/libpr0n/src/imgFrame.cpp
 block, so only NS_OK -> NS_ERROR_NOT_AVAILABLE - the rest is already in 1.9.2.12)
58c60f220da285f1847dacc839350c1828b8e40d (except for imgContainer::WriteToDecoder, as it doesn't seem exist yet, and I skipped one assert)
9a8451dad9afd4db856839eb71354e21bfa54019 (everything but comments)
dc5674a71f286c0f68b1cdaff04c8959e797a0fc (this one is that not really related part, but as this also affects animated gifs, I've took it too)

Actually, I've produced that patch with no plans of putting it here, but as it was decided it's upstreamable, I was strongly suggested to drop it here.
Comment 17 Jory A. Pratt 2010-12-01 08:19:03 PST
*** Bug 552986 has been marked as a duplicate of this bug. ***
Comment 18 Jory A. Pratt 2010-12-04 06:02:20 PST
(In reply to comment #14)
> Rafał, what are the bug numbers you backported?

bug 545513 546272 534787 are the actual bug numbers.
Comment 19 Juan Castro 2010-12-07 11:39:18 PST
I'm trying to adapt Rafał's patch to Seamonkey as we speak, but I'm a n00b. Not a C n00b, but a Mozilla code n00b. I may need help.
Comment 20 Rafał Mużyło 2010-12-07 12:33:44 PST
I never said I really know Mozilla code well.
It was simply that mercurial commits for xulrunner 2.0 are well documented
and changes between 1.9.2 and 2.0 in those parts of code are minimal (well, nearly).

Semonkey 2.0, on the other hand, is xulrunner 1.9.1, and just looking at the commit, that created imgFrame.cpp (http://hg.mozilla.org/releases/mozilla-1.9.2/rev/b94bc4be53ca) makes me unsure *if* such fix can be made in 1.9.1,
much less where it should go.
Comment 21 Martin Stránský 2010-12-14 05:52:49 PST
Joe, are you fine with the backported patch? Or should we produce something else? Or do you suggest to nominate the Bug 545513 Bug 546272 Bug 534787 to 1.9.2 w/o backporting?
Comment 22 Hicham 2010-12-27 11:43:32 PST
Mozilla's behavior in here is just weird to say the least.

"We don't have time to review the patch, and we won't give you permission to use it downstream either"

Why allow building against system libs if you aren't going to keep up with upstream changes ?
Comment 23 Jory A. Pratt 2010-12-27 11:49:03 PST
(In reply to comment #22)
> Mozilla's behavior in here is just weird to say the least.
> 
> "We don't have time to review the patch, and we won't give you permission to
> use it downstream either"
> 
Who has said you could not use it downstream?

> Why allow building against system libs if you aren't going to keep up with
> upstream changes ?

Mozilla uses bundled versions of the library, they expect distros to use the bundled while most distros do not, it is up to us to fix the breakage, the issue was already addressed in trunk where cairo was updated to 1.10.0.
Comment 24 Hicham 2010-12-27 12:08:17 PST
(In reply to comment #23)
> (In reply to comment #22)
> > Mozilla's behavior in here is just weird to say the least.
> > 
> > "We don't have time to review the patch, and we won't give you permission to
> > use it downstream either"
> > 
> Who has said you could not use it downstream?
> 

From http://www.mozilla.org/foundation/trademarks/policy.html :

"If you compile Mozilla unmodified source code (including code and config files in the installer) and do not charge for it, you do not need additional permission from Mozilla to use the relevant Mozilla Mark(s) for your compiled version."

> > Why allow building against system libs if you aren't going to keep up with
> > upstream changes ?
> 
> Mozilla uses bundled versions of the library, they expect distros to use the
> bundled while most distros do not, it is up to us to fix the breakage, the
> issue was already addressed in trunk where cairo was updated to 1.10.0.

Fixing breakages is subject to the excerpt above.
Comment 25 Daniel Holbert [:dholbert] 2010-12-27 12:51:04 PST
(In reply to comment #22)
> Mozilla's behavior in here is just weird to say the least.
> 
> "We don't have time to review the patch"

No one has said that, AFAICT.

Here's the review feedback that I'd give, though, for what it's worth -- since the attached patch is mostly a rollup of several backports (per comment 13), I'd suggest splitting it into a separate patch-file for each backported changeset (plus an additional "not-directly-backported changes" patch if necessary).

That would make it easier to see what's different in the trunk patches vs. their backported versions, thereby making review much easier.  It also would make regression-tracking & backouts (if needed) significantly saner/easier.

> and we won't give you permission to
> use it downstream either

(Of course you have permission to apply any arbitrary patches that you like. You're just not allowed to distribute the result as "Firefox" without Mozilla's permission.)
Comment 26 Daniel Holbert [:dholbert] 2010-12-27 13:00:50 PST
(In reply to comment #25)
> > and we won't give you permission to
> > use it downstream either

(Note also that no one has said "we won't give you permission".  It's rather that "you need to get Mozilla's permission [in order to label a patched build as Firefox]".)

(And for what it's worth, I think the changes I suggested in Comment 25 would make the backported code much saner and the aforementioned permission and/or review much more likely to be forthcoming.)
Comment 27 Hicham 2010-12-27 13:09:14 PST
> (Of course you have permission to apply any arbitrary patches that you like.
> You're just not allowed to distribute the result as "Firefox" without Mozilla's
> permission.)

Then you should tighten your policy about which system libs versions firefox/xulrunner can be linked to.
Comment 28 Mike Hommey [:glandium] 2010-12-28 08:56:01 PST
(In reply to comment #27)
> Then you should tighten your policy about which system libs versions
> firefox/xulrunner can be linked to.

Version checking/restriction is not a proper solution, IMHO. Having a good test suite is, and writing tests for known problems should be done (like, is there a unit test for this one? If not, there really should)
Comment 29 Martin Stránský 2011-01-04 08:10:44 PST
Bug 545513 Bug 534787 have 1.9.2 versions of the patches,  Bug 546272 applies ok to 1.9.2.
Comment 30 Daniel Veditz [:dveditz] 2011-01-07 10:23:09 PST
Adding dependencies helps keep relationships straight.
Comment 31 Daniel Holbert [:dholbert] 2011-01-07 10:23:15 PST
Comment on attachment 494277 [details] [diff] [review]
(mostly a rollup of several backports) fix for animated gif problem

I'm canceling the pending review-request here, for clarity, since it's better to take individual backported patches instead of a rollup.  (and it looks like Martin is working on getting the individual backports to be landable, on the various bugs)

Martin: Have you confirmed that the combined patches from Comment 29 fix this issue?  (Comment 13's "more or less" seemed to imply that something more might be needed...(?))
Comment 32 christian 2011-01-07 10:23:33 PST
> Bug 545513 Bug 534787 have 1.9.2 versions of the patches,  Bug 546272 applies
> ok to 1.9.2.

All bugs are approved except for bug 545513, which is waiting on review.(In reply to comment #29). If / when that gets approved, is this bug INVALID?
Comment 33 Daniel Holbert [:dholbert] 2011-01-07 10:32:06 PST
I don't know why INVALID would be the right resolution -- maybe FIXED by dependencies (or WORKSFORME)?

(I think you're asking the same question that I was asking at the end of Comment 31, though -- Martin, does this bug become fixed when the various backports are applied?)
Comment 34 Martin Stránský 2011-01-10 00:49:33 PST
Yes, a combination of patches from Bug 545513 Bug 534787 and Bug 546272 fixes this issue on 1.9.2.
Comment 35 Ionut Biru 2011-03-28 14:19:39 PDT
shouldn't this be closed now that both 3.6.x and 4.0 contain those fixes?
Comment 36 Chris 2011-03-28 16:43:25 PDT
Actually 3.6.x is not fixed, unless dropping back to an older version of cairo.

Having said that I'd yes it probably should be closed simply because we are at 4.0 now and 3.6.x days are numbered. The problem is fixed in 4.0
Comment 37 Joe Drew (not getting mail) 2011-03-28 18:16:27 PDT
3.6.16 should contain this fix.
Comment 38 Chris 2011-03-30 06:06:43 PDT
Just tried 3.6.16 in Slackware64, problem still exist.
Comment 39 Christopher Aillon (sabbatical, not receiving bugmail) 2011-04-09 15:24:07 PDT
It actually will be fixed in 3.6.17.  3.6.16 was pushed out with a single fix for the comodo issue, and everything previously slated for .16 is now going to be in .17 instead.
Comment 40 Marc-Jano Knopp 2011-07-28 11:45:37 PDT
{ Hopefully, this comment automagically re-opens this bug }

I still have that problem with the animated GIFs
in the original bugreport and with other sites.

I'm using Iceweasel (Firefox) 3.5.19-3 with a fresh profile
in Debian testing (AMD64), together with libcairo2-1.10.2-6.
Comment 41 Matt McCutchen 2011-07-28 12:09:39 PDT
I doubt Mozilla is interested in fixing this on the 3.5 branch.  Encourage your distributor to move to a newer branch.

Note You need to log in before you can comment on or make changes to this bug.