Last Comment Bug 743598 - GIF animation gets stuck when src and image size are changed
: GIF animation gets stuck when src and image size are changed
Status: RESOLVED FIXED
: regression, relnote, testcase
Product: Core
Classification: Components
Component: General (show other bugs)
: 11 Branch
: x86_64 Linux
: -- normal with 1 vote (vote)
: mozilla14
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on: 721659
Blocks: 666446
  Show dependency treegraph
 
Reported: 2012-04-08 17:23 PDT by panzi
Modified: 2012-07-30 14:33 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
+
wontfix
+
wontfix
+
fixed


Attachments
example demonstrating animation getting stuck (41.75 KB, text/html)
2012-04-08 17:23 PDT, panzi
no flags Details
example demonstrating animation getting stuck (41.73 KB, text/html)
2012-04-08 18:00 PDT, panzi
no flags Details
screen recording demonstrating the bug (173.80 KB, video/ogg)
2012-04-08 18:17 PDT, panzi
no flags Details

Description panzi 2012-04-08 17:23:38 PDT
Created attachment 613209 [details]
example demonstrating animation getting stuck

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.19 (KHTML, like Gecko) Chrome/18.0.1025.151 Safari/535.19

Steps to reproduce:

I wrote a JavaScript animation that changes the src and style (width and height) attributes of GIF images. The new src is another animated GIF.

I tested it with these two setups:

(1
Firefox user agent string:
Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0

Installed from Les RPM de Remi - Packages:
http://rpms.famillecollet.com/

OS:
Fedora Linux 14 x86_64

(2
Firefox user agent string:
Mozilla/5.0 (Windows NT 5.1; rv:11.0) Gecko/20100101 Firefox/11.0

Installed from:
http://www.mozilla.org/

OS:
Windows XP SP 3 in a VirtualBox running in the Linux mentioned before


Actual results:

The GIF animation gets stuck sometimes when the src attribute and style (width and height) are changed.


Expected results:

Animation playback without errors.
Comment 1 panzi 2012-04-08 17:26:51 PDT
Comment on attachment 613209 [details]
example demonstrating animation getting stuck

Click back and forth between the two buttons in the attached file. After the second or third click it gets stuck for me.

The buttons change the src attribute and the width and height styles. I used data:-urls in the example just so everything is included in the html file. The same thing occurs when it's a normal url to a gif on a web server.
Comment 2 panzi 2012-04-08 17:32:02 PDT
FYI: A user of this silly little thing I wrote noticed the bug first:
http://web.student.tuwien.ac.at/~e0427417/browser-ponies/ponies.html

See the bug report to my script:
https://bitbucket.org/panzi/browser-ponies/issue/6/animation-stopping-and-ponies-sliding

I implemented a workaround by simple replacing the whole image element when the src needs to be changed (if the browser is Gecko based).
Comment 3 Daniel Holbert [:dholbert] 2012-04-08 17:38:47 PDT
(In reply to panzi from comment #0)
> Steps to reproduce:
> 
> I wrote a JavaScript animation that changes the src and style (width and
> height) attributes of GIF images. The new src is another animated GIF.

Could you provide actual steps that someone can follow to (hopefully) trigger the bug?  The more detailed the better.  It's OK if the steps don't trigger the bug 100% of the time -- I just want to be sure I & others are following roughly the same procedure as you.

(As noted in bug 691792 comment 22, I haven't yet been able to reproduce, from madly clicking back and forth between the buttons, using Nightly as well as the Ubuntu-provided Firefox 11 package on Linux x86.)

Also -- if it's possible to construct a testcase that automatically triggers the bug (no mysterious clicking required), that would be amazing.

e.g. it looks like you could have <html onload="go()"> where "go()" calls showFly / showDash a bunch of times with setTimeouts. (enough to trigger the bug)
Comment 4 panzi 2012-04-08 18:00:28 PDT
Created attachment 613211 [details]
example demonstrating animation getting stuck

This example automatically changes the src attribute and style 3 times, after which the animation gets stuck. (Tested again in both mentioned setups.)
Comment 5 panzi 2012-04-08 18:17:24 PDT
Created attachment 613213 [details]
screen recording demonstrating the bug
Comment 6 Daniel Holbert [:dholbert] 2012-04-08 18:19:32 PDT
[Oops, sorry -- when I thought I was using Ubuntu's Firefox 11 package, I was in fact still using Nightly.  Sorry for my mistake on that. In any case, thanks for the automatic testcase -- it's much easier to test with!]

I can reproduce with both attached testcases, using...
 - Firefox 11 (from Ubuntu)
 - current Firefox Beta (12) and Aurora (13), just-downloaded

However, it's fixed in Nightly.
Comment 7 panzi 2012-04-08 18:23:18 PDT
Great! :)
Comment 8 Daniel Holbert [:dholbert] 2012-04-08 18:30:51 PDT
I tracked down the fix range on trunk.
Last bad nightly: 2012-03-20
First good nightly: 2012-03-21

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b972b89518c3&tochange=4bdae514b9be

This changeset looks the most promising/related, from that range:
0f6ef3a02450	Boris Zbarsky — Bug 721659. Update the width/height of an image synchronously when src is set to something that was preloaded. r=bholley

I also verified that Firefox 10 is unaffected:
  Mozilla/5.0 (X11; Linux i686; rv:10.0.2) Gecko/20100101 Firefox/10.0.2
So this is a new regression in Firefox 11.

As a result, it looks like this isn't an exact duplicate of bug 721659, since (per bug 721659 comment 4) that bug affects builds at least as old as Firefox 9.  This just is a bug that happens to be fixed by that bug's patch.

I wonder if that means we might want to backport that bug's patch, to fix this regression...?
Comment 9 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-04-08 18:38:07 PDT
I'd be a bit worried about backporting that fix.

If this regressed between 10 and 11, what's the range on that?  Would be good to know what regressed it...
Comment 10 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-04-08 18:39:08 PDT
Or to put it another way, it sounds like there's some sort of race here that leads to bad behavior in some cases.  The fix for bug 721659 changes the timing, so might affect who wins the race in a given testcase, but there still seems to be a fundamental underlying problem.
Comment 11 Daniel Holbert [:dholbert] 2012-04-08 18:42:06 PDT
(In reply to Boris Zbarsky (:bz) from comment #9)
> If this regressed between 10 and 11, what's the range on that?  Would be
> good to know what regressed it...

It's a regression from bug 666446.

Last good nightly: 2011-11-10
First bad nightly: 2011-11-11

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c60535115ea1&tochange=1374294a6119
Comment 12 Alex Keybl [:akeybl] 2012-04-09 15:40:45 PDT
Unless there is a near-zero risk fix or backout for this bug, the user pain here does not appear significant enough to introduce unnecessary risk for FF12. Tomorrow (4/10) is the latest we'd be willing to take a fix for that release.
Comment 13 Daniel Holbert [:dholbert] 2012-04-09 15:55:58 PDT
(In reply to Alex Keybl [:akeybl] from comment #12)
> Unless there is a near-zero risk fix or backout for this bug

Per comment 9, the patch that fixed this on trunk has nonzero risk, and a backout (of bug 666446) would be huge & have nonzero risk as well.

> the user pain
> here does not appear significant enough to introduce unnecessary risk for
> FF12.

Agreed.
Comment 14 Daniel Holbert [:dholbert] 2012-04-09 15:59:02 PDT
I suspect this bug is worth mentioning in the "known issues" of Firefox 12 release notes, since it's a regression (w.r.t. Firefox 10).

Adding "relnote" keyword (hopefully that's the right thing to do).
Comment 15 Alex Keybl [:akeybl] 2012-05-02 14:58:47 PDT
(In reply to Daniel Holbert [:dholbert] from comment #13)
> (In reply to Alex Keybl [:akeybl] from comment #12)
> > Unless there is a near-zero risk fix or backout for this bug
> 
> Per comment 9, the patch that fixed this on trunk has nonzero risk, and a
> backout (of bug 666446) would be huge & have nonzero risk as well.

Sounds like the same logic applies to FF13 in that case. We'll wait for FF14 to see this fixed.
Comment 16 Daniel Holbert [:dholbert] 2012-07-18 13:45:05 PDT
Firefox 14 (which includes the patch for bug 721659) has now been released, so this bug is now fixed in all releases.  (Note that ESR 10.* should also be unaffected by this bug, as this was a regression in Firefox 11 (per the middle of comment 8 and comment 11))

So I don't think there's any reason to leave this bug open at this point.

Resolving as FIXED by bug 721659. (Note that it's not an exact dupe, per the end of comment 8)

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