Right now there is e10s-specific code in place to support click-to-play plugins in XUL fennec, but this approach won't work for fennec native or desktop Firefox, so it needs to be changed. This bug is just about adding the basic support for click-to-play, and we can work in follow-up bugs to create more fine-tuned preferences like the ones described in bug 549697. Also, since it seems like some of these preferences are only wanted on desktop, we shouldn't block on them for mobile. I still need to add a way to pref this on/off, but this WIP patch works (tested on desktop with a slimmed down version of the front-end patch in bug 549697).

Comment on attachment 579245 [details] [diff] [review] WIP Josh, what do you think of this approach? Also will your patch in bug 90268 affect this work, apart from bit-rot?

I'll add the mobile UI in another bug, but this patch includes a pref that disables this by default. Tested on desktop, flash plays normally.

Comment on attachment 579810 [details] [diff] [review] patch +nsObjectLoadingContent::PlayPlugin() +{ + mHasBeenClickedToPlay = true; + return LoadObject(mURI, true, mContentType, true); This as written lets a webpage QI an embed or what not to nsIObjectLoadingContent and call playPlugin() (separate bug, but no fix for that in the near future), which I don't think we want to permit. I'd say we should check that the caller if this method is chrome, and if that's not the case do nothing. IOW, ad a check for nsContentUtils::IsCallerChrome() at the very top of this method and return NS_OK if that returns false. r=jst with that fixed.

Backed out due to bustage :( https://hg.mozilla.org/integration/mozilla-inbound/rev/5b4a90374698 I'll try fixing it and push to try server instead of mozilla-inbound.

So, the problem was caused by setting the default value of aHasBeenClickedToPlay to false in the methods I changed in nsPluginHost. Changing that to true fixed the bustage, since that enables the plugins as expected. However, that also breaks the click to play functionality, so I'm going to have to invest some more effort in this to fix it :(

How about: nsresult IsPluginEnabledForType(const char* aMimeType) { return IsPluginEnabledForType(aMimeType, !mozilla::Preferences::GetBool("plugins.click_to_play", false)); } nsresult IsPluginEnabledForExtension(const char* aExtension, const char* &aMimeType) { return IsPluginEnabledForExtension(aExtension, aMimeType, !mozilla::Preferences::GetBool("plugins.click_to_play", false)); } nsresult IsPluginEnabledForType(const char* aMimeType, bool aHasBeenClickedToPlay); nsresult IsPluginEnabledForExtension(const char* aExtension, const char* &aMimeType, bool aHasBeenClickedToPlay);

pushed that to try https://tbpl.mozilla.org/?tree=Try&rev=645ad4589609

Brad, is this what you meant? It works for me locally (testing on desktop), but I'm going to push it to try to make sure it doesn't cause any bustage. I also updated the variable names from aHasBeenClickedToPlay/mHasBeenClickedToPlay to aShouldPlay/mShouldPlay, since that's less confusing. And I moved the pref from firefox.js to all.js, since that's where it should have been in the first place.

(In reply to Brad Lassey [:blassey] from comment #8) > pushed that to try https://tbpl.mozilla.org/?tree=Try&rev=645ad4589609 Oops, I didn't notice this. I pushed my patch to try also: https://tbpl.mozilla.org/?tree=Try&rev=424b32fb6529.

(In reply to Brad Lassey [:blassey] from comment #8) > pushed that to try https://tbpl.mozilla.org/?tree=Try&rev=645ad4589609 this ran green

pushed https://hg.mozilla.org/mozilla-central/rev/bc84b3376e14

Click not working.

(In reply to Andrew from comment #13) > Click not working. If you're on mobile, this might be from bug 710885. For desktop, see bug 711552 (the UI hasn't landed, so you'll need a patch to enable it).