Closed
Bug 1350381
Opened 8 years ago
Closed 8 years ago
Change Flash Blocking to use the user setting in about:addons for CTA
Categories
(Core Graveyard :: Plug-ins, defect)
Core Graveyard
Plug-ins
Tracking
(firefox53 fixed, firefox54 fixed, firefox55 fixed)
VERIFIED
FIXED
mozilla55
People
(Reporter: bytesized, Assigned: bytesized)
References
Details
(Whiteboard: [fce-active-legacy])
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
qdot
:
review+
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
|
Details |
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•8 years ago
|
No longer depends on: flashstudy
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8851698 -
Flags: review?(kyle)
Comment 2•8 years ago
|
||
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?
Updated•8 years ago
|
Flags: needinfo?(ksteuber)
Assignee | ||
Comment 3•8 years 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)
Comment 4•8 years ago
|
||
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•8 years ago
|
Attachment #8851698 -
Flags: review?(kyle)
Assignee | ||
Updated•8 years 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•8 years ago
|
Attachment #8851698 -
Flags: feedback?(benjamin)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8851698 -
Flags: feedback?(benjamin)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8851698 -
Flags: feedback?(benjamin)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8851698 -
Flags: feedback?(benjamin)
Comment 9•8 years ago
|
||
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•8 years 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•8 years ago
|
Attachment #8851698 -
Flags: review?(kyle)
Comment 12•8 years ago
|
||
mozreview-review |
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•8 years 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•8 years 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•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/da5282f55afb
Status: NEW → RESOLVED
Closed: 8 years 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•8 years 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).
Comment 18•8 years ago
|
||
Note: to uplift this to beta without conflicts we gotta uplift the _test-only_ bug 1340448 (no need to request approval for that)
Comment 19•8 years ago
|
||
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•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/1edf2dfa3809
Comment 21•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/14b6ab023dec
Comment 22•8 years 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 24•8 years ago
|
||
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•8 years 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
Updated•7 years ago
|
Whiteboard: [fce-active] → [fce-active-legacy]
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•