Closed
Bug 597174
Opened 14 years ago
Closed 14 years ago
Firefox 3.6.10 doesn't render gifs correctly, w/ certain system-cairo versions. Displays (flashes) only the first frame.
Categories
(Core :: Graphics: ImageLib, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: firew4lker, Assigned: galtgendo)
References
Details
Attachments
(1 file)
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.
No extensions loaded. Using "nouveau" display drivers on xorg 1.9 Linux.
Comment 3•14 years ago
|
||
happens when using cairo 1.10.0. with 1.8.10 is fine
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•14 years ago
|
||
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
I hope mozilla won't suggest to use the bundled cairo for gecko-1.9.x
Comment 8•14 years ago
|
||
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•14 years ago
|
||
Related / dup-candidate: bug 552986 .
Comment 10•14 years ago
|
||
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•14 years ago
|
||
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•14 years ago
|
||
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
Assignee | ||
Comment 13•14 years ago
|
||
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
Updated•14 years ago
|
Component: General → ImageLib
Product: Firefox → Core
QA Contact: general → imagelib
Updated•14 years ago
|
Attachment #494277 -
Flags: review?(jmuizelaar)
Updated•14 years ago
|
Attachment #494277 -
Flags: review?(jmuizelaar) → review?(joe)
Comment 14•14 years ago
|
||
Rafał, what are the bug numbers you backported?
Assignee: nobody → galtgendo
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 15•14 years ago
|
||
(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.
Assignee | ||
Comment 16•14 years ago
|
||
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 18•14 years ago
|
||
(In reply to comment #14)
> Rafał, what are the bug numbers you backported?
bug 545513 546272 534787 are the actual bug numbers.
Comment 19•14 years ago
|
||
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.
Updated•14 years ago
|
Summary: Firefox 3.6.10 doesn't render gifs correctly. Displays (flashes) only the first frame. → Firefox 3.6.10 doesn't render gifs correctly, w/ certain system-cairo versions. Displays (flashes) only the first frame.
Assignee | ||
Comment 20•14 years ago
|
||
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•14 years ago
|
||
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•14 years ago
|
||
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•14 years ago
|
||
(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•14 years ago
|
||
(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•14 years ago
|
||
(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•14 years ago
|
||
(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•14 years ago
|
||
> (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•14 years ago
|
||
(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•14 years ago
|
||
Comment 30•14 years ago
|
||
Adding dependencies helps keep relationships straight.
Comment 31•14 years ago
|
||
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...(?))
Attachment #494277 -
Attachment description: fix for animated gif problem → (mostly a rollup of several backports) fix for animated gif problem
Attachment #494277 -
Flags: review?(joe)
Comment 32•14 years ago
|
||
> 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•14 years ago
|
||
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?)
Updated•14 years ago
|
Version: unspecified → 1.9.2 Branch
Comment 34•14 years ago
|
||
Comment 35•14 years ago
|
||
shouldn't this be closed now that both 3.6.x and 4.0 contain those fixes?
Comment 36•14 years ago
|
||
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•14 years ago
|
||
3.6.16 should contain this fix.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WORKSFORME
Comment 38•14 years ago
|
||
Just tried 3.6.16 in Slackware64, problem still exist.
Comment 39•14 years ago
|
||
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•13 years ago
|
||
{ 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•13 years ago
|
||
I doubt Mozilla is interested in fixing this on the 3.5 branch. Encourage your distributor to move to a newer branch.
Version: 1.9.2 Branch → 1.9.1 Branch
Updated•13 years ago
|
Version: 1.9.1 Branch → 1.9.2 Branch
You need to log in
before you can comment on or make changes to this bug.
Description
•