Last Comment Bug 756367 - gif playback restarts once a JS-inserted image is fully loaded
: gif playback restarts once a JS-inserted image is fully loaded
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: 13 Branch
: x86_64 Windows 7
: -- normal (vote)
: mozilla16
Assigned To: Robert Lickenbrock [:rclick]
:
Mentors:
http://dl.dropbox.com/u/123900/gifpla...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-17 21:58 PDT by Steven
Modified: 2012-06-14 09:57 PDT (History)
5 users (show)
ryanvm: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (5.05 KB, patch)
2012-06-11 08:04 PDT, Robert Lickenbrock [:rclick]
bzbarsky: review+
Details | Diff | Review
patch for check-in (5.07 KB, patch)
2012-06-13 12:19 PDT, Robert Lickenbrock [:rclick]
rclickenbrock: review+
Details | Diff | Review

Description Steven 2012-05-17 21:58:47 PDT
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20100101 Firefox/13.0
Build ID: 20120509070325

Steps to reproduce:

Load a page containing an animated .gif that is dynamically inserted into the page using `document.createElement('img')` and appending the Element to the body.


Actual results:

The gif animation will restart from the beginning as soon as it is fully loaded (fires the `onload` callback), even if the playback had advanced with the downloaded-part of the image. This resetting of playback often happens in the middle of the image, which is annoying.

.gifs loaded in regular image tags do not display this behavior.

This effect can be seen at:

http://dl.dropbox.com/u/123900/gifplaybackreset.html


Expected results:

Once a dynamically inserted .gif becomes fully loaded, it should keep playing from the currently displayed frame.
Comment 1 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-05-23 13:21:37 PDT
I'm not seeing the problem with a Mac nightly, if I understand the problem description correctly....
Comment 2 Steven 2012-05-23 13:57:07 PDT
I took a video of the effect:

http://www.youtube.com/watch?v=HOa313-WRDA

The second and third videos have an onload handler that changes the opacity of the image back to 1, so the image appears faded until it fully loads. While the gif in the <img> tag loads fully, playback continues. In the dynamically loaded image, playback restarts from the beginning. If this didn't happen, the playback of all three images would be nearly in sync even after all three were loaded.

Another interesting effect is that when the dynamically loaded image's src is the same as the <img> tags, then the animation reset occurs on all the images:

http://dl.dropbox.com/u/123900/gifplaybackreset_2.html

I can additionally reproduce this bug on Firefox 11, on the same system (Windows 7 x64).
Comment 3 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-05-23 14:02:32 PDT
Ah, ok.  I can get this to happen, sometimes.  Weird....
Comment 4 Steven 2012-05-23 14:07:00 PDT
The effect is best seen with a clear cache, so the download time and delta between playback is significant. I always reload my example pages from the server with Ctrl+Shift+R.
Comment 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-05-23 14:21:10 PDT
Ctrl+Shift+R might affect behavior here: it marks the images as uncacheable entirely (including in the image cache, iirc), so if anything requires a possible reload of the image it will be refetched from the server.

The only sane way to test the behavior here is with a clean profile for each test....
Comment 6 Steven 2012-05-23 14:59:30 PDT
I wrote a really simple userscript that also displays this bug:

https://gist.github.com/2778012 (click on 'raw' to install with GM)

With the userscript installed using Greasemonkey or Scriptish, browse to http://www.reddit.com/r/gifs/ and hover the mouse over one of the thumbnails. The full .gif will be dynamically inserted into the document, with the same opacity change onload. You can see the playback restart effect on most of the images.

You can use the same userscript in Chrom{e|ium}, and the playback does not restart when the image loads.

(In reply to Boris Zbarsky (:bz) from comment #5)
> Ctrl+Shift+R might affect behavior here: it marks the images as uncacheable
> entirely (including in the image cache, iirc), so if anything requires a
> possible reload of the image it will be refetched from the server.
> 
> The only sane way to test the behavior here is with a clean profile for each
> test....

I don't think Ctrl+Shift+R effects the behavior beyond making the time between initially displaying the image and the onload event long enough to observe the effect. The effect still occurs even with cached images (watch for the little flash of reduced opacity in one of the tests), but it's less annoying and noticeable.
Comment 7 David Adler 2012-05-28 13:04:14 PDT
I'm seeing this bug as described in Firefox 12.0 and Nightly 15.0a1 (2012-05-27) on Intel Mac OSX 10.6.8, and also in a difference scenario: 

A pre-release version of my add-on "Thumbail Zoom Plus" shows a pop-up image in an <html:img> element.  The element itself is statically defined in chrome's overlay.xul but its src URL is set dynamically from javascript.  The gif starts playing before it is completely loaded as expected, but restarts playing from the beginning as soon as loading completes.  So the user sees the beginning of the animation play twice, which is bad.

I could work around it by not displaying the image until I get onload, but that would make the user wait longer before it starts playing.
Comment 8 David Adler 2012-05-28 14:14:22 PDT
The problem happens even if the <img> tag is directly in the html rather than JS-inserted, but the src is set dynamicall from JS.  eg:

<img id="dynamic-img" style="">                    
<script>
document.addEventListener( 'DOMContentLoaded', function () {
        var img = document.getElementById('dynamic-img');                              
        img.src = 'http://dl.dropbox.com/u/123900/more_quality.gif';                  
        img.style.opacity = 0.5;                                                     
        img.addEventListener( 'load', function () { 
                                 this.style.opacity = '';
                               })                                                                        
});                                                                            
</script>
Comment 9 Robert Lickenbrock [:rclick] 2012-06-11 08:04:41 PDT
Created attachment 631897 [details] [diff] [review]
patch

So, it turns out that we're waiting until OnStopDecode to reset the animation after the .src changes, when it should really be happening sooner than that.

This patch changes that so the animation is reset when we add a new current request, or when a pending request becomes the current request.
Comment 10 Joe Drew (not getting mail) 2012-06-11 12:37:29 PDT
Comment on attachment 631897 [details] [diff] [review]
patch

Bobby, feel free to steal this from Boris.
Comment 11 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-06-11 20:45:32 PDT
Comment on attachment 631897 [details] [diff] [review]
patch

r=me
Comment 12 Robert Lickenbrock [:rclick] 2012-06-13 12:19:25 PDT
Created attachment 632810 [details] [diff] [review]
patch for check-in

Updated commit message for check-in. Carrying forward r=bz.
Comment 13 Ryan VanderMeulen [:RyanVM] 2012-06-13 18:16:26 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/66b45adcc4c5

Should this have a test?
Comment 14 Ed Morley [:emorley] 2012-06-14 02:45:32 PDT
https://hg.mozilla.org/mozilla-central/rev/66b45adcc4c5
Comment 15 Robert Lickenbrock [:rclick] 2012-06-14 06:39:11 PDT
This probably should have a test, but I'm having trouble figuring out a way to test it.

Joe, do you have any ideas? I looked into having an image with a non-looping animation and a listener that implements imgIDecoderObserver/imgIContainerObserver which tracks how many times frameChanged is called, but that won't work because frameChanged is noscript...
Comment 16 Joe Drew (not getting mail) 2012-06-14 09:57:29 PDT
You could use imgIContainerDebug, which has an attribute, framesNotified, you can look at.

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