Change Flash Blocking to use the user setting in about:addons for CTA

VERIFIED FIXED in Firefox 53

Status

()

Core
Plug-ins
VERIFIED FIXED
4 months ago
2 months ago

People

(Reporter: bytesized, Assigned: bytesized)

Tracking

(Depends on: 1 bug)

unspecified
mozilla55
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox53 fixed, firefox54 fixed, firefox55 fixed)

Details

(Whiteboard: [fce-active])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

4 months ago
The Flash blocking blacklists should be usable even when Flash blocking is disabled. This behavior will be accessible via a pref.

This feature will be needed for eventual deployment of Flash blocking, but will not be needed for the Shield study.
(Assignee)

Updated

4 months ago
No longer depends on: 1335232

Updated

4 months ago
Blocks: 1348089
Whiteboard: [fce-active]
Comment hidden (mozreview-request)
(Assignee)

Updated

4 months ago
Attachment #8851698 - Flags: review?(kyle)
Is it possible to change the behavior of the existing pref, rather than introduce a new one? In particular, the UI we want still doesn't map very well to the available pref options here:

Whitelist contents                    WHITELISTED     BLACKLISTED     OTHER
about:addons
"never activate"                      deny            deny            deny
"ask to activate"
* "use mozilla lists" checked/default allow           deny            ask
* "use mozilla lists" unchecked       ask             ask             ask
"always activate"
* "use mozilla lists" checked/default allow           deny            allow
* "use mozilla lists" unchecked       allow           allow           allow

I think you could get the desired behavior here by alternately enabling plugins.flashBlock.blacklist.forceEnable for some cases and plugins.flashBlock.enabled for other cases. However it's not a clean mapping.

Why though can't we just have the plugins.flashBlock.enabled pref turn on the whitelist and blacklist behavior without changing the default behavior at all?
Flags: needinfo?(ksteuber)
(Assignee)

Comment 3

4 months ago
In the current implementation (which I feel is the best way to do this), the Flash blocking system introduces additional security, but never detracts from existing security. This means that I never implemented a whitelist mechanism that "breaks through" other security to allow automatic activation of whitelisted content despite security settings.

That is to say, all the whitelist does is prevents the site from being CTA'ed by the Flash blocking system itself. It does not prevent it from being CTA'ed by the "Ask to Activate" setting.
Flags: needinfo?(ksteuber)
ok, but... in the addon manager UI, when this is deployed to users, the setting that is displayed to users will be "ask to activate". And in reality, what's happening is that this feature is tuning ask-to-activate to stop asking in cases where we don't think the user 1) needs to make a choice or 2) can make an informed choice.

As long as the UI ends up in comment 2, the underlying prefs are an engineering choice, but I still find these prefs hard to read and reason about when structured this way.
(Assignee)

Updated

4 months ago
Attachment #8851698 - Flags: review?(kyle)
(Assignee)

Updated

4 months ago
Summary: Allow Flash blacklist to be activated without Flash Blocking enabled → Change Flash Blocking to use the user setting in about:addons for CTA
Comment hidden (mozreview-request)
(Assignee)

Updated

4 months ago
Attachment #8851698 - Flags: feedback?(benjamin)
Comment hidden (mozreview-request)
(Assignee)

Updated

4 months ago
Attachment #8851698 - Flags: feedback?(benjamin)
Comment hidden (mozreview-request)
(Assignee)

Updated

4 months ago
Attachment #8851698 - Flags: feedback?(benjamin)
Comment hidden (mozreview-request)
(Assignee)

Updated

4 months ago
Attachment #8851698 - Flags: feedback?(benjamin)
Comment on attachment 8851698 [details]
Bug 1350381 - Change Flash blocking to allow the setting "Ask to Activate" to control CTA of unknown documents.

This is a great commit message!
Attachment #8851698 - Flags: feedback?(benjamin) → feedback+
(Assignee)

Comment 10

4 months ago
It looks like this patch is causing failures in browser/base/content/test/plugins/browser_CTP_context_menu.js. Once I fix that I will request review for this patch.
Comment hidden (mozreview-request)
(Assignee)

Updated

4 months ago
Attachment #8851698 - Flags: review?(kyle)
Comment on attachment 8851698 [details]
Bug 1350381 - Change Flash blocking to allow the setting "Ask to Activate" to control CTA of unknown documents.

https://reviewboard.mozilla.org/r/123944/#review127348
Attachment #8851698 - Flags: review?(kyle) → review+

Comment 13

4 months ago
Pushed by ksteuber@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/da5282f55afb
Change Flash blocking to allow the setting "Ask to Activate" to control CTA of unknown documents. r=qdot
(Assignee)

Comment 14

4 months ago
Comment on attachment 8851698 [details]
Bug 1350381 - Change Flash blocking to allow the setting "Ask to Activate" to control CTA of unknown documents.

Approval Request Comment
[Feature/Bug causing the regression]: This feature represents a significant change to the mechanism underlying Flash blocking, which will be undergoing a Shield study on Release 52 (Bug 1335232).
[User impact if declined]: The Shield study will test the current mechanism instead of the new one
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Manual testing available at itisatrap.org
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Minimal Risk
[Why is the change risky/not risky?]: Code changes are pref'ed off.
[String changes made/needed]: none
Attachment #8851698 - Flags: approval-mozilla-beta?
Attachment #8851698 - Flags: approval-mozilla-aurora?

Comment 15

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/da5282f55afb
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox55: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment on attachment 8851698 [details]
Bug 1350381 - Change Flash blocking to allow the setting "Ask to Activate" to control CTA of unknown documents.

As I understand it this should change behavior only for the shield study, for a study on 53 (not for 52)
Attachment #8851698 - Flags: approval-mozilla-beta?
Attachment #8851698 - Flags: approval-mozilla-beta+
Attachment #8851698 - Flags: approval-mozilla-aurora?
Attachment #8851698 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 17

4 months ago
Sorry about that! It should have read:

[Feature/Bug causing the regression]: This feature represents a significant change to the mechanism underlying Flash blocking, which will be undergoing a Shield study on Release 53 (Bug 1335232).
Blocks: 1352224
Note: to uplift this to beta without conflicts we gotta uplift the _test-only_ bug 1340448 (no need to request approval for that)
https://hg.mozilla.org/releases/mozilla-aurora/rev/004cd303c040b4390fc6c60e5b3b26e34d21d944
https://hg.mozilla.org/releases/mozilla-beta/rev/f975e688b2289c2a590ef87c8fa80bd9db844a0b
status-firefox53: --- → fixed
status-firefox54: --- → fixed

Comment 20

4 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/1edf2dfa3809

Comment 21

4 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/14b6ab023dec

Comment 22

4 months ago
Pushed by philringnalda@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/43983c66642e
Define this pref so that the test doesn't fail. r=me a=test-fix

Comment 23

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/43983c66642e
Note: this made it to the Nightly on Mar 31, and the build ID I got for that on Mac is 20170331030216 (this will need to be set as the minBuildID for bug 1352224)

Comment 25

3 months ago
This bug has been verified on Windows 7/10, Ubuntu 16.04 and Mac OS X 10.11 on fixed versions. When changing the options in about:addons to "always activate" or "ask to activate" it shows flash as expected.
Status: RESOLVED → VERIFIED
Blocks: 1365714
You need to log in before you can comment on or make changes to this bug.