Closed
Bug 1364972
Opened 7 years ago
Closed 7 years ago
Allow WebExtensions to disable animated images
Categories
(WebExtensions :: General, enhancement, P5)
WebExtensions
General
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
Assignee | ||
Comment 1•7 years ago
|
||
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).
Assignee | ||
Comment 2•7 years ago
|
||
(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)
Assignee | ||
Comment 3•7 years ago
|
||
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.)
Comment 4•7 years ago
|
||
(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)
Comment 5•7 years ago
|
||
> 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.
Assignee | ||
Comment 6•7 years ago
|
||
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)
Assignee | ||
Comment 7•7 years ago
|
||
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)
Comment 8•7 years ago
|
||
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)
Comment 9•7 years ago
|
||
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
Updated•7 years ago
|
Flags: needinfo?(amckay)
Updated•7 years ago
|
Flags: needinfo?(amckay)
Assignee | ||
Comment 10•7 years ago
|
||
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]
Comment 11•7 years ago
|
||
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#
Updated•7 years ago
|
Priority: P2 → P5
Whiteboard: triaged[browserSettings][design-decision-needed] → triaged[browserSettings][design-decision-approved]
Comment hidden (mozreview-request) |
Comment 13•7 years ago
|
||
mozreview-review |
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+
Assignee | ||
Comment 14•7 years ago
|
||
mozreview-review-reply |
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.
Assignee | ||
Comment 15•7 years ago
|
||
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.
Comment 16•7 years ago
|
||
Pushed by bsilverberg@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/674775dda8ad Allow WebExtensions to disable animated images, r=mixedpuppy
Backed out for eslint issues like https://treeherder.mozilla.org/logviewer.html#?job_id=129320124&repo=autoland https://hg.mozilla.org/integration/autoland/rev/9047e2077ec73734bdf76d60856d0a1c99bef64b
Flags: needinfo?(bob.silverberg)
And some android xpcshell failures for good measure: https://treeherder.mozilla.org/logviewer.html#?job_id=129322469&repo=autoland
Comment hidden (mozreview-request) |
Comment 20•7 years ago
|
||
Pushed by bsilverberg@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/39a2a87fc100 Allow WebExtensions to disable animated images, r=mixedpuppy
Comment 21•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/39a2a87fc100
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(bob.silverberg)
Assignee | ||
Updated•7 years ago
|
Keywords: dev-doc-needed
Comment 22•7 years ago
|
||
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)
Assignee | ||
Comment 23•7 years ago
|
||
(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)
Comment 24•7 years ago
|
||
Thanks for the catch, Bob, I made that change.
Keywords: dev-doc-needed → dev-doc-complete
Comment 25•6 years ago
|
||
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)
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(bob.silverberg) → qe-verify-
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•