Closed Bug 909589 Opened 6 years ago Closed 6 years ago

image.animation_mode=once does not work sometimes

Categories

(Core :: ImageLib, defect, major)

26 Branch
x86_64
All
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla31
Tracking Status
e10s later ---
firefox26 --- affected
firefox27 --- affected
firefox28 --- affected
firefox29 --- affected
firefox-esr17 --- unaffected
firefox-esr24 --- unaffected

People

(Reporter: mats, Assigned: rclick)

References

(Blocks 1 open bug, )

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

STEPS TO REPRODUCE
1. go to about:config and set image.animation_mode to "once" (without the quotes)
2. restart Firefox
3. load http://www.obviouswinner.com/obvwin/2013/8/20/shes-got-those-creepy-painted-on-anime-eyes.html

ACTUAL RESULTS
The animated GIF loops.  If the problem doesn't occur, try Shift+Reload.
I can reproduce the problem also in "View Image" mode (from the context menu).

EXPECTED RESULTS
The animated GIF should play once then stop.
Shift+Reload should play it once then stop.

PLATFORM AND BUILDS TESTED
Bug occurs in Nightly 26.0a1 (2013-08-26) on Linux64.
(Built from http://hg.mozilla.org/mozilla-central/rev/14b1e8c2957e)

It might be a regression; this is the first time I've noticed a problem
for several years of using this pref.
This is totally a regression, and I also noted this bug on the Windows 64 nightly, and am able to repro with the OP's animated GIF on a new profile.

There may be more to that. Do steps 1 and 2, then 
3. Load http://en.wikipedia.org/w/index.php?title=Banach%E2%80%93Tarski_paradox&oldid=581840803
4. Observe the first picture under the heading "A sketch of the proof" is animated. If it does not, refresh. It never took me more than one refresh to get it animated non-stop. Also note that it does naimate for me every time I open the link for the first time in a newly created profile, attempted twice here.
5. Open the GIF in new tab: right-click on it, then click on "View Image" in the context menu while holding down Ctrl key.
6. The GIF in the new tab will be animated, wierdly looking like a negative and on transparent bacground, for the first iteration of animation, then will animate non-stop in normal colors.
7. Go back to the original page, and note that animation has stopped. Looks like it stops when the View Image command is executed.

Similarly, the animation in the original page stops when Context Menu/Ctrl+View Image... is done on the image on the page referred to by the bug poster. The image opened in the new tab remains animated the first time you open it, but the animation stops in the image embedded in the page.

This is on 26.0 beta channel, updated with the latest build just minutes ago. User agent string is "Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0", firefox.exe version 26.0.0.5066.
This bug started happening to me when I upgraded to FF 26.  I have it set to "once", but GIF animations are looping.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Same here. Right after the update from 25.1 to 26.0. No add-ons were upgraded. So it must be FF. (Windows 7, 64bit)
And here on Linux 64-bit. The upgrade from 25.0.1 to 26 broke image.animation_mode = once
Now animated GIFs loop endlessly.
Also occurs on x86.

Folks, this bug has already been confirmed.  PLEASE DO NOT POST HERE UNLESS YOU HAVE SUBSTANTIAL NEW INFORMATION.
Whiteboard: PLEASE DO NOT POST "ME TOO" COMMENTS; this is already confirmed
Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/5976b9c673f8
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20130715 Firefox/25.0 ID:20130715211256
Bad:
http://hg.mozilla.org/mozilla-central/rev/41ed26826acb
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20130716 Firefox/25.0 ID:20130716015911
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5976b9c673f8&tochange=41ed26826acb

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/bf847b1a3776
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20130715 Firefox/25.0 ID:20130715113446
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/ea8d855c4edb
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20130715 Firefox/25.0 ID:20130715113946
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=bf847b1a3776&tochange=ea8d855c4edb

Regressed by:
ea8d855c4edb	Joe Drew — Bug 717872 - Move all image animation logic into a new class, FrameAnimator, and use it from RasterImage. r=seth This patch moves the logic of moving from one frame to another (and tracking what frame is current, etc) to a separate class, FrameAnimator. Deciding *whether* to animate, and actually calling that animation code, is left to RasterImage, but the animation itself is driven by FrameAnimator.


And, only fixed in Beta25.0.
Bad
http://hg.mozilla.org/releases/mozilla-beta/rev/143e8c67cf1b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 ID:20130917123208
Fixed:
http://hg.mozilla.org/releases/mozilla-beta/rev/7d23197d995a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 ID:20130923194050
Pushlog:
http://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=143e8c67cf1b&tochange=7d23197d995a

Fixed in Firefox25.0beta
	ef213b06605e	Milan Sreckovic — Bug 899861 - Animated gifs should not wait to play until fully downloaded. This is a partial backout of 717872 with the intent to re-enable using the separate FrameAnimator class. Hide the new approach behind #define USE_FRAME_ANIMATOR for now. r=bgirard, a=akeybl
OS: Linux → All
Version: Trunk → 26 Branch
Severity: normal → major
Duplicate of this bug: 950330
Duplicate of this bug: 952748
I suspect this could be because FrameAnimator's constructor unconditionally sets mAnimationMode to kNormalAnimMode... Can we add a call `mAnim->setAnimationMode(mAnimationMode)` just below RasterImage.cpp's `mAnim = new FrameAnimator(mFrameBlender);`? I'd try it, but I don't have a working build environment.
Flags: needinfo?(seth)
Simon's suggestion fixes the bug for me in Seamonkey 2.24:

--- comm-release-orig/mozilla/image/src/RasterImage.cpp 2014-02-04 16:09:51 +0900
+++ comm-release/mozilla/image/src/RasterImage.cpp      2014-03-10 03:53:30 +0900
@@ -1077,6 +1077,7 @@
 
     // Create the animation context
     mAnim = new FrameAnimator(mFrameBlender);
+    mAnim->SetAnimationMode(mAnimationMode);
 
     // We don't support discarding animated images (See bug 414259).
     // Lock the image and throw away the key.
This is a slight variation of Simon's suggestion that passes the animation mode into FrameAnimator's constructor. I haven't tried it yet (I have other things on my to-do list at the moment) but it builds locally.

Pushed to try as https://tbpl.mozilla.org/?tree=Try&rev=7864a07c1f05
Builds will be at https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/rclickenbrock@gmail.com-7864a07c1f05
Assignee: nobody → rclickenbrock
Status: NEW → ASSIGNED
(In reply to Robert Lickenbrock [:rclick] from comment #11)
> This is a slight variation of Simon's suggestion that passes the animation
> mode into FrameAnimator's constructor. I haven't tried it yet (I have other
> things on my to-do list at the moment) but it builds locally.

Very nice! Just tried this out and it seems to fix the problem here.

Could you request review from me so that we can get this landed?
Flags: needinfo?(seth) → needinfo?(rclickenbrock)
Attachment #8388229 - Flags: review?(seth)
Flags: needinfo?(rclickenbrock)
Mass tracking-e10s flag change. Filter bugmail on "2be0fcce-e36a-4e2c-aa80-0e3d33eb5406".
tracking-e10s: --- → +
Comment on attachment 8388229 [details] [diff] [review]
Ensure FrameAnimator is created with the correct animation mode

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

Thanks Robert. Looks good.
Attachment #8388229 - Flags: review?(seth) → review+
Formatted for checkin. There's a more recent try run at [1]. The parent changeset wasn't exactly all green, but there doesn't appear to be any new failures.

[1] https://tbpl.mozilla.org/?tree=Try&rev=7a7780435308
Attachment #8388229 - Attachment is obsolete: true
Attachment #8413043 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/88f7fddea805
Keywords: checkin-needed
Whiteboard: PLEASE DO NOT POST "ME TOO" COMMENTS; this is already confirmed
https://hg.mozilla.org/mozilla-central/rev/88f7fddea805
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Firefox 30, Linux.  I verified in about:config that image.animation_mode=once

Still not working properly.  Example:  When I go to this page:  http://www.zggtr.org/index.php?topic=17334.0  the second post has an animated GIF avatar which never stops animating.  The element is http://www.zggtr.org/index.php?action=dlattach;attach=2554;type=avatar filename of avatar_12_1308578025.gif

If I leave the tab and then come back to it, the animation stops.  And if I close the tab and reopen it from a parent link, it doesn't animate at all.  And if I right click on the image and use "view image" it also doesn't animate at all.  If I force a full reload with shift-<click on reload icon> then it will start animating FOREVER again, as long as you don't leave the tab/window.  In all those cases, it should have looped through the 6 frames ONCE when it was initially displayed and then stopped.

So it is still animating when it shouldn't, and not animating (once through) when it should.  I don't see how this is "fixed".
> I don't see how this is "fixed".

The patches are not in Firefox 30.  The "Target Milestone" 'way up at the top tells you which version the fix will be in -- in this case, Firefox 31.  If the schedule is what I think it is, that should be the Beta release real soon now.
(In reply to Zack Weinberg (:zwol) from comment #19)

> The patches are not in Firefox 30.  The "Target Milestone" 'way up at the
> top tells you which version the fix will be in -- in this case, Firefox 31. 
> If the schedule is what I think it is, that should be the Beta release real
> soon now.

Ooops, silly me.  Sorry about that :)  I saw the tracking flags in the EMail going up to status-firefox29:affected and thought it meant 30 was untested.
Verified as fixed using the following environment:

FF 31.02
Build Id:20140616143923
OS: Windows 7 x64, Ubuntu 12.10 x32, Mac Os X 10.8.5
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.