Closed Bug 1364972 Opened 7 years ago Closed 7 years ago

Allow WebExtensions to disable animated images

Categories

(WebExtensions :: General, enhancement, P5)

enhancement

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: bsilverberg, Assigned: bsilverberg)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: triaged[browserSettings][design-decision-approved])

Attachments

(1 file)

This will use browserSetting [1] to manage which extension has control over the pref. This does not really belong in privacy so will go into the browserSettings API.

According to the notes in [2], this would make use of the `image.animation_mode` preference, which seems to allow the following values [3]:

normal, none, once

Therefore it would make sense to support the same 3 values for the setting browser.browserSettings.imageAnimationBehavior.

[1] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/privacy/BrowserSetting
[2] https://docs.google.com/document/d/1HZiI-dqFATzLpmTtN1Bg6tD8y4N0Ne3D74LEWWgttOk/edit#
[3] http://searchfox.org/mozilla-central/source/layout/base/nsPresContext.cpp#628-638
This comment is from an email from Justin Dolske to the "Intent to Implement" thread:

------

This one probably needs some more thought. Many "animated images" are
really just HTML5 video now, so I don't know that it's terribly useful to
expose this old capability (which only works for GIF). For that matter, it
seems like a curious omission that there's nothing in this document related
to HTML5 audio/video (especially around autoplay).
(In reply to Justin Dolske from comment #1)
> 
> This one probably needs some more thought. Many "animated images" are
> really just HTML5 video now, so I don't know that it's terribly useful to
> expose this old capability (which only works for GIF). For that matter, it
> seems like a curious omission that there's nothing in this document related
> to HTML5 audio/video (especially around autoplay).

Rob, can you please comment on this?
Flags: needinfo?(rob)
Adding another comment from the email thread, from Simon Lindholm:

-----

Speaking as (biased) author of the
https://addons.mozilla.org/en-US/firefox/addon/toggle-animated-gifs/
addon (with ~20k users, and which I'm hoping to port to WebExtensions at
some point soon):
This is definitely useful. Animated GIFs are still used all over, for
instance for icons, on old forums, sometimes reddit, etc. (My
personal pet peeve is the phpbb "active topic" icon.) HTML5 video can
already be controlled via content script -- video.pause(),
video.play(), "loadedmetadata" event, etc. -- while there are no such
APIs for plain images (especially not for CSS images!), nor do I
imagine there ever will be.
Also, far from all videos are replacements for animated images
("GIFV"s). My addon currently has a small set of rules to detect those
and prevent them from starting, while still allowing, say, YouTube to
autoplay. Agreed that general APIs for controlling audio/video autoplay
would still be useful for other cases, though.
(I've been meaning to propose fine-grained web extension APIs for image
animation control, corresponding to the old XPCOM APIs, but haven't had
time yet. For instance it's very nice to be able to toggle animation on
and off dynamically, preferably on an image-by-image basis.)
(In reply to Bob Silverberg [:bsilverberg] from comment #2)
> (In reply to Justin Dolske from comment #1)
> > 
> > This one probably needs some more thought. Many "animated images" are
> > really just HTML5 video now, so I don't know that it's terribly useful to
> > expose this old capability (which only works for GIF). For that matter, it
> > seems like a curious omission that there's nothing in this document related
> > to HTML5 audio/video (especially around autoplay).
> 
> Rob, can you please comment on this?

Currently there is no API to distinguish animated images from static images, let alone blocking the former. It is also difficult to disable animations through other means (~ detect all potential images, paint on <canvas> and replace image with serialized canvas).

I did not mention HTML5 audio/video in the request because these can already be controlled via media element APIs in content scripts, and blocked at the network level with the webRequest extension API.
Flags: needinfo?(rob)
> blocked at the network level with the webRequest extension API.

Since extension can be anything (not just .mp4 for example), would this be done by blocking specific MIME types? I wonder if a <video> tag will still play if the MIME type is incorrect or missing. If so, this seems hard to block with webRequest. I think we should test this.
From the comments above it does sound like introducing this API, although limited, will still be of use to extension authors. I just want to confirm that if we do it, we understand the correct implementation.

Justin, as someone who commented on the original email, would you be able to tell us whether flipping the `image.animation_mode` pref via a WebExtensions API will give us the behaviour we expect? Are there other things we need to consider for this limited API? If you are not the correct person to answer these questions, might you be able to direct me to someone more appropriate?
Flags: needinfo?(dolske)
Jared, could you please take a look at comment #6, above and let me know if you have answers for the questions I posed there? If not, can you suggest someone else to ask?
Flags: needinfo?(dolske) → needinfo?(jaws)
Unfortunately I'm not very experienced with this preference. Bug 17686 added this preference, and I haven't seen any use of it outside of the extension referenced in comment #3 and similar extensions.

The lack of an image-specific or domain-specific API is unfortunate. I agree with Dolske that it would be nice if the API would cover other types of media so extension authors don't have to implement media-autoplaying using multiple approaches.

To answer your original question, it does appear that flipping that pref will provide the behavior you expect. I confirmed locally that no restart is required when changing the pref, but images will need to be reloaded for the pref to take effect on them.
Flags: needinfo?(jaws)
See Also: → 1378939
Seems like Chrome has an API for this: https://developer.chrome.com/apps/accessibilityFeatures#property-animationPolicy (added in https://bugs.chromium.org/p/chromium/issues/detail?id=3690 )

Basically:
chrome.accessibilityFeatures.animationPolicy.set({'value': 'allowed' / 'once' / 'none'})
chrome.accessibilityFeatures.animationPolicy.get({}, policy => console.log(policy.value))
chrome.accessibilityFeatures.animationPolicy.clear({})

Used by https://chrome.google.com/webstore/detail/animation-policy/ncigbofjfbodhkaffojakplpmnleeoee
Flags: needinfo?(amckay)
Flags: needinfo?(amckay)
This request has been sitting here for quite some time. There seems to be some disagreement about whether simply implementing this as is, with just this one pref being controlled, is a good idea. Let's come to a final decision about whether we want to implement this at an upcoming design-decision-needed triage.
Whiteboard: triaged[browserSettings] → triaged[browserSettings][design-decision-needed]
This has been added to the agenda for the August 22 WebExtensions APIs triage meeting. Anyone who is interested in this bug is welcome to join! 

Wiki: https://wiki.mozilla.org/Add-ons/Contribute/Triage#Next_Meeting

Agenda: https://docs.google.com/document/d/11SdY-aRhvPU3SvH8jpj0covj3Teq9_GJl8wMeEeSVwo/edit#
Priority: P2 → P5
Whiteboard: triaged[browserSettings][design-decision-needed] → triaged[browserSettings][design-decision-approved]
Comment on attachment 8905485 [details]
Bug 1364972 - Allow WebExtensions to disable animated images,

https://reviewboard.mozilla.org/r/177296/#review182382

::: toolkit/components/extensions/ext-browserSettings.js:8
(Diff revision 1)
>  "use strict";
>  
>  XPCOMUtils.defineLazyModuleGetter(this, "ExtensionSettingsStore",
>                                    "resource://gre/modules/ExtensionSettingsStore.jsm");
>  XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
>                                    "resource://gre/modules/Preferences.jsm");

Lets change this to Services.prefs, perhaps in a followup bug.
Attachment #8905485 - Flags: review?(mixedpuppy) → review+
Comment on attachment 8905485 [details]
Bug 1364972 - Allow WebExtensions to disable animated images,

https://reviewboard.mozilla.org/r/177296/#review182382

> Lets change this to Services.prefs, perhaps in a followup bug.

Thanks for the review, Shane. Yes, we do need to convert this to Services.prefs, but it won't be entirely straightforward as there is some code in here that relies on the abstractions provided by Preferences.jsm, so a follow-up bug is a good idea. I will file one.
Mark opened bug 1390611 about ext-privacy.js and I've added ext-browserSettings.js and ExtensionPreferencesManager.jsm to that bug as files to convert to Services.prefs.
Pushed by bsilverberg@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/674775dda8ad
Allow WebExtensions to disable animated images, r=mixedpuppy
Pushed by bsilverberg@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/39a2a87fc100
Allow WebExtensions to disable animated images, r=mixedpuppy
https://hg.mozilla.org/mozilla-central/rev/39a2a87fc100
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Flags: needinfo?(bob.silverberg)
Keywords: dev-doc-needed
I've added a page on this: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/browserSettings/imageAnimationBehavior

Please let me know if this covers it.
Flags: needinfo?(bob.silverberg)
(In reply to Will Bamberg [:wbamberg] from comment #22)
> I've added a page on this:
> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/
> browserSettings/imageAnimationBehavior
> 
> Please let me know if this covers it.

Looks good, except the first bullet point reads: "normal": the default. Play animated images as normal. The default.

I think you may want to remove one of those pieces that say "the default".
Flags: needinfo?(bob.silverberg)
Thanks for the catch, Bob, I made that change.
Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.
Flags: needinfo?(bob.silverberg)
Flags: needinfo?(bob.silverberg) → qe-verify-
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.