1275591 - Enable plugin content blocking by default

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Tobias Schneider [:tobytailor]]

This is for flipping browser.safebrowsing.blockedURIs.enabled.

Comment 1

[Tobias Schneider [:tobytailor]]

Attachment: Flip browser.safebrowsing.blockedURIs.enabled

Comment 2

[Benjamin Smedberg]

safebrowsing is really the pref name? That's weird; does this have anything to do with the safe browsing system?

Comment 3

[Tobias Schneider [:tobytailor]]

Yes, we are taking the safe browsing path for the url classification.

Comment 4

[Chris Peterson [:cpeterson]]

[Tracking Requested - why for this release]:

Ritu, just a heads up: this bug is the plugin blocklist feature we talked about enabling in 47.0b9. Tobias will land this pref in Nightly 49 and then ask to uplift to Aurora 48 and Beta 47.0b9.

Comment 5

[Benjamin Smedberg]

Comment on attachment 8756393 [details] [diff] [review]
Flip browser.safebrowsing.blockedURIs.enabled

Yes, ship it.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa77bc23f799

Comment 7

[Chris Peterson [:cpeterson]]

Comment on attachment 8756393 [details] [diff] [review]
Flip browser.safebrowsing.blockedURIs.enabled

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: No change in the amount of plugin content loaded.
[Describe test coverage new/current, TreeHerder]: The plugin blocklist has been tested on the Beta 47 channel during our 10% beta experiment.
[Risks and why]: Medium risk. The plugin blocklist is a pretty short list and focused on nonessential plugin content. If a website is broken, we can publish a new (or empty) list on the shavar server without requiring a Firefox hotfix.
[String/UUID change made/needed]: No string or UUID changes.

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8756393 [details] [diff] [review]
Flip browser.safebrowsing.blockedURIs.enabled

At the Tuesday channel meeting this week, we decided to enable this pref on by default in Fx47. We also have good mitigation plans in place in case this becomes an issue after we ship. Aurora48+, Beta47+

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Chris, please nominate this bug for relnote addition whenever the blog post and other links are ready. I'd like to add it to the Fx47 release notes (to be reviewed) early next week if possible. Thanks!

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This was backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/33e51331fe0359998d75b909258b602b63f8cac9 for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=28813760&repo=mozilla-inbound so I can't uplift it.

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi guys, can you please help address test failures asap? I am trying to get this included in 47.0b9. I really do not want to include this in RC1 but rather land this today!

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

47.0b9 build is already in progress, we will not have an updated patch ready soon enough. I would prefer not to uplift this in RC1. CPeterson sent out an email to r-d that we will not be making this change in Fx47 but targeting Fx48 instead.

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8756393 [details] [diff] [review]
Flip browser.safebrowsing.blockedURIs.enabled

Reverting the A+ on this patch. We may need an updated patch that fixes the test failures.

Comment 14

[Chris Peterson [:cpeterson]]

Tobias is debugging the test failures so we can land in Nightly 49.

Comment 15

[Benjamin Smedberg]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc5cf5d091f9

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d4289b95a79
Enable plugin content blocking by default

Comment 17

[Benjamin Smedberg]

Comment on attachment 8756393 [details] [diff] [review]
Flip browser.safebrowsing.blockedURIs.enabled

Approval Request Comment - this needs to land with https://bugzilla.mozilla.org/show_bug.cgi?id=1268120#c18 - see approval comment there

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4d4289b95a79

Comment 19

[Carsten Book [:Tomcat]]

sorry had to back this out from m-c and all integration branches since we have perma failures like https://treeherder.mozilla.org/logviewer.html#?job_id=31839256&repo=mozilla-inbound after this landing

Comment 20

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/4c05938a64a7
Backed out changeset 4d4289b95a79 for causing m-2  Windows 8 x64 opt perma failures in test_bug659071.html

Comment 21

[Chris Peterson [:cpeterson]]

Tobias, can you take a look at these plugin mochitests failures?

TEST-UNEXPECTED-FAIL | dom/plugins/test/mochitest/test_bug1165981.html | uncaught exception - TypeError: pluginElement.identifierToStringTest is not a function at run@http://mochi.test:8888/tests/dom/plugins/test/mochitest/test_bug1165981.html:51:12

Comment 22

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8756393 [details] [diff] [review]
Flip browser.safebrowsing.blockedURIs.enabled

Not taking it to 48 (just like bug 1268120)

Comment 23

[Gerry Chang [:gchang]]

Comment on attachment 8756393 [details] [diff] [review]
Flip browser.safebrowsing.blockedURIs.enabled

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

This patch enabled plugin blocklist feature. Take it in aurora.

Comment 24

[Gerry Chang [:gchang]]

Comment on attachment 8756393 [details] [diff] [review]
Flip browser.safebrowsing.blockedURIs.enabled

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

Didn't notice this was backout.

Comment 25

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8756393 [details] [diff] [review]
Flip browser.safebrowsing.blockedURIs.enabled

If this is about uplifting to 49, flipping from aurora:? to beta:? flag post-merge.

Comment 26

[Chris Peterson [:cpeterson]]

Comment on attachment 8756393 [details] [diff] [review]
Flip browser.safebrowsing.blockedURIs.enabled

I am temporarily resetting the uplift requests because the original pref-enabling patch was backed out from mozilla-inbound (comment 19). Once Tobias lands the test fix (bug 1290749), he can try landing this patch again. If Nightly 51 looks good, then he can re-request uplift to Aurora or Beta.

Comment 27

[Chris Peterson [:cpeterson]]

Tobias, your test fix (bug 1290749) landed in mozilla-central, so you can now try to reland this patch to enable the blocklist pref.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0bd5218a807
Enable plugin content blocking by default. r=bsmedberg

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f0bd5218a807

Comment 30

[Liz Henry (:lizzard) (relman/hg->git project)]

Want to request uplift to beta for this to go with bug 1290749?

Comment 31

[Chris Peterson [:cpeterson]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #30)
> Want to request uplift to beta for this to go with bug 1290749?

Yes. The test fix in bug 1290749 *must* land before this bug's patch. AFAICT, Tobias already requested both approval-mozilla-aurora and approval-mozilla-beta for the test fix in bug 1290749 comment 6.

Comment 32

[Chris Peterson [:cpeterson]]

Comment on attachment 8756393 [details] [diff] [review]
Flip browser.safebrowsing.blockedURIs.enabled

The Flash blocklist test fix bug 1290749 has been uplifted to Aurora 50 and Beta 49, so we'd like to uplift this patch to enable the Flash blocklist feature.

Approval Request Comment
[Feature/regressing bug #]: Not a regression. This patch enables the Flash blocklist feature.
[User impact if declined]: Some sites will still have invisible Flash pixels that slow down Firefox without providing any direct user benefit.
[Describe test coverage new/current, TreeHerder]: The Flash blocklist has been enabled on Nightly 51 for one week.
[Risks and why]: We're trying to reduce the risk of the Flash blocklist by restricting the first phase to a short list of Flash pixels that are not required for any major sites.
[String/UUID change made/needed]: No strings.

Comment 33

[Chris Peterson [:cpeterson]]

Release Note Request (optional, but appreciated)
[Why is this notable]: This bug enables the first phase of our Flash blocklist, part of an ongoing initiative to reduce NPAPI plugin usage.
[Suggested wording]: Block some nonessential Flash plugin content.
[Links (documentation, blog post, etc)]: https://blog.mozilla.org/futurereleases/2016/07/20/reducing-adobe-flash-usage-in-firefox/

Comment 34

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8756393 [details] [diff] [review]
Flip browser.safebrowsing.blockedURIs.enabled

Flash blocklist, I'd like to see how this works in beta as soon as possible. It should make it into beta 5.

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ccb0adb99c96

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0417cb2c8a38

Comment 37

[Eric Shepherd [:sheppy]]

Getting ready to mention this on MDN but am slightly confused by the tracking flags. Is this fixed in 51, or earlier? Or has it been rolled into patches to older builds? Or…?

Comment 38

[Chris Peterson [:cpeterson]]

(In reply to Eric Shepherd [:sheppy] from comment #37)
> Getting ready to mention this on MDN but am slightly confused by the
> tracking flags. Is this fixed in 51, or earlier? Or has it been rolled into
> patches to older builds? Or…?

The plugin content blocking shipped in Firefox 49. The feature landed in Nightly 51, but was uplifted to Beta 49 (comment 35).

Comment 39

[Eric Shepherd [:sheppy]]

OK, so this needs to be documented as a Firefox 49 change then. Thanks!

Comment 40

[Eric Shepherd [:sheppy]]

Updated Firefox 49 for developers and https://developer.mozilla.org/en-US/Add-ons/Plugins/Scripting_Plugins_Adobe_Flash.

Comment 41

[Sylvestre Ledru [:Sylvestre]]

Too late for the release notes, sorry.

Comment 42

[Sylvestre Ledru [:Sylvestre]]

Actually, I changed my mind and added it to the 49 release notes for posterity