1132427 - Badly rendered GIF on imgur

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect, P4)

Description

[Steve England [:stevee]]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0 ID:20150211222327 CSet: 3094601af679

1. New profile, start firefox.
2. Navigate to http://imgur.com/gallery/rZ4FQoK

Expected:
- Properly rendered GIF

Actual:
- Malrendered GIF. After last frame the GIF doesn't reset back to the first frame cleanly.

Strangely if you view the image directly at http://i.imgur.com/rZ4FQoK.gif then it looks fine, it is just misrendered when viewing it at http://imgur.com/gallery/rZ4FQoK

Fanolian's capture shows the problem with the rendering if you are unable to observe it: http://gfycat.com/BelovedTameAcornbarnacle

Not sure if this is a regression..

Comment 1

[Alice0775 White]

[Tracking Requested - why for this release]:

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b3a1efe7900a&tochange=59ceeca79341

Regressed by: Bug 1130328

Comment 2

[Alice0775 White]

I can reproduce on windows7 without HWA. And I can also reproduce on ubuntu with blocked HWA.

Comment 3

[Alice0775 White]

I can also reproduce with regardless of HWA on/off.

Comment 4

[Emanuel Hoogeveen [:ehoogeveen]]

(In reply to Steve England [:stevee] from comment #0)
> Strangely if you view the image directly at http://i.imgur.com/rZ4FQoK.gif
> then it looks fine, it is just misrendered when viewing it at
> http://imgur.com/gallery/rZ4FQoK

The version embedded by BugzillaJS also exhibits the problem.

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Jet, AFAIK, Seth is in PTO.
We are in a bad position right now. Bug 1130328 was supposed to fix a bunch of regression but introduced this bug.

Matt, as you did the review of bug 1130328, can you help with this? Thanks

Comment 6

[Matt Woodrow (:mattwoodrow)]

I'll look at this today.

Comment 7

[Timothy Nikkel (:tnikkel)]

Attachment: patch

The first frame refresh area is just being calculating wrong. It only gets updated for the second frame because that code was wrongly inside the "this image has become animated" if. We need to update it for every frame of the image, not just the second.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8563748 [details] [diff] [review]
patch

Review of attachment 8563748 [details] [diff] [review]:
-----------------------------------------------------------------

Add a test please.

Comment 9

[Timothy Nikkel (:tnikkel)]

Working on a test now, it's a little tricky because exposing this bug requires a looping gif. But given the time pressure on getting this fix into beta I pushed it now.

https://hg.mozilla.org/integration/mozilla-inbound/rev/109c66867717

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Indeed! thanks guys for the reactivity.
Timothy, could you fill the uplift request to aurora & beta right now? Thanks

Comment 11

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8563748 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1130328 exposed this problem. However http://hg.mozilla.org/mozilla-central/rev/d5462de77e9f bug 1079627 added the incorrect code.
[User impact if declined]: looping animated gifs will draw incorrectly
[Describe test coverage new/current, TreeHerder]: none yet, writing a test as I write this
[Risks and why]: this should be very low risk, the code is clearly wrong.
[String/UUID change made/needed]: none

Comment 12

[Emanuel Hoogeveen [:ehoogeveen]]

Oddly, I do not see this problem on Firefox 36 beta 8 or the 2015-02-13 Aurora build.

On Aurora, the image either does not animate at all or *doesn't loop* (Hawking shows up, then never goes away). I can fix this by going to the direct link and force reloading (Ctrl + F5). After that, the image animates correctly both directly and viewed through imgur, and does not exhibit this problem. I'm guessing the animation problems have to do with bug 1119938 and the patches fixing it that are still waiting for uplift.

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/109c66867717

Comment 14

[Ryan VanderMeulen [:RyanVM]]

This needs rebasing for uplift to both branches.

Comment 15

[Timothy Nikkel (:tnikkel)]

So it turns out the buggy code isn't on beta or aurora. It's only on nightly, since the buggy code came from bug 1079627, which has target milestone of 38.

No comments in this bug indicate that they can see the bug in aurora or beta. So this is probably a nightly only bug. If anyone can see this bug on beta please speak up, but it will most likely have a different cause and require a different patch.

Comment 16

[Steve England [:stevee]]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0 ID:20150214030238 CSet: a7c177546ca0

Now WFM.

Comment 17

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

I can confirm this works fine now in the latest Firefox 38 Nightly (Windows 7 x64). It was reproducible in Firefox 38 Nightly from February 11th. 

Also the issue does not show for the latest Firefox 37 Aurora (had to Ctrl+Shift+R for the gif to animate) and 36 Beta 9. I'm adjusting the flags accordingly if no one minds.

Comment 18

[Timothy Nikkel (:tnikkel)]

And the test
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8dc15f1f923

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b8dc15f1f923

Comment 20

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8563748 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: 1079627
[User impact if declined]: Issues with animated GIF rendering.
[Describe test coverage new/current, TreeHerder]: We have a test.
[Risks and why]: Low risk - this is a very small fix.
[String/UUID change made/needed]: None.

Comment 21

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8563748 [details] [diff] [review]
patch

Trivial fix for a regression that simply corrects how the if statement is enclosed. Aurora+

Comment 22

[Seth Fowler [:seth] [:s2h]]

Pushed to Aurora:

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/4a8edebbc3ce
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/f23746928e84

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Backed out.
https://hg.mozilla.org/releases/mozilla-aurora/rev/0d2ff4249817

Comment 24

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8563748 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: Bug fix for bug 1079627.
[User impact if declined]: Incorrect animated GIF rendering.
[Describe test coverage new/current, TreeHerder]: In 38 for a long time. Has a test.
[Risks and why]: Very low risk; in 38 for a long time. Was already approved for uplift to 37 when 37 was Aurora, but was backed out as collateral damage from an unrelated backout, even though there were no problems with this patch.
[String/UUID change made/needed]: None.

Comment 25

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8563748 [details] [diff] [review]
patch

OK. Let's try this again. Beta+ this time.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0d4ecc5c742c
https://hg.mozilla.org/releases/mozilla-beta/rev/97f402db19a4

Comment 27

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

A quick check on Windows 7 x64 using Firefox 37 Beta 6 does not show this issue.