STR: 1. Somehow get a plugin to be click-to-play blocklisted (either roll your own blocklist.xml or install an old plugin that's currently blocked by default, etc.) 2. set extensions.blocklist.enabled to false 3. visit a page with the above plugin Expected results: the plugin plays Actual results: the plugin is click-to-play (non-security version) Right now, the clicktoplay flag in nsPluginTags is set by nsBlocklistService when a plugin is ctp-blocklisted. When nsBlocklistService is disabled, however, this flag isn't unset. This has two problems. The first is that nsPluginHost will see that flag and say a plugin is ctp when it shouldn't be. The second is that the clicktoplay flag can't be used if/when we move from all-or-nothing user-set click-to-play to plugin-by-plugin user-set click-to-play. I think this flag is ill-suited for this purpose. Instead of using the flag, nsPluginHost should just ask nsBlocklistService the blocklist state of a plugin in IsPluginClickToPlayForType. If it's one of the click-to-play blocklist states or if plugins.click_to_play is true, it will return true.

Here's what I think we should do. Let me know if this doesn't make sense or will cause problems. In terms of priority, this isn't huge, but it would probably be a good idea to fix before we really let this loose on the world.

As it turns out, when switching channels (e.g. current -> aurora or nightly -> current), a similar thing happens where a plugin will retain its click-to-play status regardless of whether or not the plugin blocklist entry applies to that version of firefox. So, this is a bit more than just a good idea - we should really fix this sooner rather than later.

Clearly a potential candidate for uplift to Fx 18.

Here's the try run: https://tbpl.mozilla.org/?tree=Try&rev=d847655597ac I added another check to test_pluginBlocklistCtp.js, but it's minor, so I'm carrying over the r+. Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/41a0c591a156

Comment on attachment 683309 [details] [diff] [review] patch v1.1 [Approval Request Comment] Bug caused by (feature/regressing bug #): click-to-play blocklisting plugins User impact if declined: essentially, we won't be confident in deploying any click-to-play blocks Testing completed (on m-c, etc.): see try run, above Risk to taking this patch (and alternatives if risky): low String or UUID changes made by this patch: I added a method to nsIPluginHost - I can work around this by adding nsIPluginHost2, if that's the way we think we should go

Comment on attachment 683309 [details] [diff] [review] patch v1.1 [Triage Comment] Approving for Aurora/Beta. The merge will be complete in a few minutes, you can land at that time.

Pushed to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/5b7a7ab98fca Pushed to beta: https://hg.mozilla.org/releases/mozilla-beta/rev/09e13b703139

Whoops - shouldn't have closed this until it hit central.

Verified fixed on Nightly 20.0a1 (2012-11-21) Win 7 x64.

Thanks Paul. Can you please check this on that latest Firefox 18 Beta and 19 Aurora?

Verified fixed on: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0b1 Aurora 19.0a2 (2012-11-21).

Paul, it would be great if you could also check that the scenarios in https://bugzilla.mozilla.org/show_bug.cgi?id=810984#c10 work as expected. Thanks!

1. Start FF 17 with a new profile, ping -> Flash not blocked, Java softblocked 2. Start FF 18 with the same profile, ping -> Flash CTP blocked, Java CTP blocked This works fine as expected. But trying vice-versa: 1. Start FF 18 with a new profile, ping -> Flash CTP blocked, Java CTP blocked 2. Start FF 17 with the same profile, ping -> Flash not blocked, Java NOT softblocked So here is the problem - Java is not softblocked when downgrading from FF 18 to 17

Whatever the final resolution is here, it will need to be uplifted to the ESR as well (which also has CTP).

(In reply to Paul Silaghi [QA] from comment #15) > 1. Start FF 17 with a new profile, ping -> Flash not blocked, Java > softblocked > 2. Start FF 18 with the same profile, ping -> Flash CTP blocked, Java CTP > blocked > This works fine as expected. Awesome - thanks for checking that. > But trying vice-versa: > 1. Start FF 18 with a new profile, ping -> Flash CTP blocked, Java CTP > blocked > 2. Start FF 17 with the same profile, ping -> Flash not blocked, Java NOT > softblocked > So here is the problem - Java is not softblocked when downgrading from FF 18 > to 17 I figured that might be the case. I believe this is a bug that has been in the blocklist code since it was written, basically. Since we're moving to click-to-play (and it's almost done, for real this time, I swear), I'm not sure how important it is we fix that. I'll start work on getting this fix to ESR tomorrow.

My understanding is that IID changes can't happen on ESR, so I had to add nsIPluginHost2. Josh - I'd appreciate if you would have a look to make sure I did that properly. (The other option is to leave nsIPluginHost as it is and basically not test some aspects of plugin click-to-play blocklisting, but I'm assuming we don't want to do that.)

Comment on attachment 686251 [details] [diff] [review] esr17 patch [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: see user impact User impact if declined: click-to-play blocklisting plugins won't work properly on (esr)17 without this (it all depends on when users upgrade, but this is a bad situation to be in, so we should just fix it) Fix Landed on Version: 18 Risk to taking this patch (and alternatives if risky): lowish (it's possible I broke nsPluginHost, but I ran this through try (https://tbpl.mozilla.org/?tree=Try&rev=786b11645825), and despite android not working, the other platforms were mostly green) String or UUID changes made by this patch: no changes - I did add nsIPluginHost2 See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Setting extensions.blocklist.enabled to false disable click-to-play blocklisting. Verified fixed FF 17.0.2 ESR Win 7 x64.

David, what about comment 17? Should be fixed in FF 18 and 17.0.2 ESR ?

Well, this will probably never work when downgrading to 17, if that's what you're asking. But yes, this should be fixed in FF 18 and whatever ESR corresponds to 18.