Firefox 3.6.10 doesn't render gifs correctly, w/ certain system-cairo versions. Displays (flashes) only the first frame.

RESOLVED WORKSFORME

Status

()

Core
ImageLib
RESOLVED WORKSFORME
7 years ago
6 years ago

People

(Reporter: Alexander, Assigned: Rafał Mużyło)

Tracking

1.9.2 Branch
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
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

7 years ago
Exact same problem here.

Comment 2

7 years ago
No extensions loaded. Using "nouveau" display drivers on xorg 1.9 Linux.

Comment 3

7 years ago
happens when using cairo 1.10.0. with 1.8.10 is fine
(Reporter)

Comment 4

7 years ago
I am having the issue with cairo-lcd 1.8.10
(Reporter)

Comment 5

7 years ago
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

7 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

Comment 7

7 years ago
I hope mozilla won't suggest to use the bundled cairo for gecko-1.9.x

Comment 8

7 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

7 years ago
Related / dup-candidate: bug 552986 .

Comment 10

7 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

7 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

7 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

7 years ago
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
Component: General → ImageLib
Product: Firefox → Core
QA Contact: general → imagelib

Updated

7 years ago
Attachment #494277 - Flags: review?(jmuizelaar)
Attachment #494277 - Flags: review?(jmuizelaar) → review?(joe)
Rafał, what are the bug numbers you backported?
Assignee: nobody → galtgendo
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 15

7 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

7 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.

Updated

7 years ago
Duplicate of this bug: 552986

Comment 18

7 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

7 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.
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

7 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

7 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

7 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

7 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

7 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.
(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.)
(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

7 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.
(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

7 years ago
Bug 545513 Bug 534787 have 1.9.2 versions of the patches,  Bug 546272 applies ok to 1.9.2.
Adding dependencies helps keep relationships straight.
Depends on: 545513, 546272, 534787
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

7 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?
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?)
Version: unspecified → 1.9.2 Branch

Comment 34

7 years ago
Yes, a combination of patches from Bug 545513 Bug 534787 and Bug 546272 fixes this issue on 1.9.2.

Comment 35

6 years ago
shouldn't this be closed now that both 3.6.x and 4.0 contain those fixes?

Comment 36

6 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
3.6.16 should contain this fix.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WORKSFORME

Comment 38

6 years ago
Just tried 3.6.16 in Slackware64, problem still exist.
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

6 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

6 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

6 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.