We'll be wanting this on desktop for sure, maybe Madhava and Shorlander can have a draw-off (or dance-off) for the visual designs. ;-)
Pending actual mockups (if required here), this is a tri-state pref where, on android, we'll pop up a radio dialog with the three options:
( ) Tap to Enable
( ) No
*** Bug 707178 has been marked as a duplicate of this bug. ***
Oh, I misunderstood this. Bug 549697 is doing the click-to-play stuff, this bug is just some basic prefs so doesn't really matter for Desktop. Though, fwiw, 549697 is implementing support in this for addons-mgr, I'm not sure how much of that Fennec gets for free. And since plugins are a special case on Fennec anyway (just Flash, right), that might not even be useful.
(In reply to Justin Dolske [:Dolske] from comment #4)
> Oh, I misunderstood this. Bug 549697 is doing the click-to-play stuff, this
> bug is just some basic prefs so doesn't really matter for Desktop. Though,
> fwiw, 549697 is implementing support in this for addons-mgr, I'm not sure
> how much of that Fennec gets for free. And since plugins are a special case
> on Fennec anyway (just Flash, right), that might not even be useful.
I've been trying to sort out the dependencies on bug 549697, and we definitely need the basic platform support that's in progress in that bug in order to get this working on mobile. However, the WIP platform patch in bug 549697 is trying to do more than we need on mobile (and more than I think we really need in a first step for desktop), so I think I will try to file separate smaller bugs that can break up that work. Stay tuned!
(AIUI, the addons manager part of bug 549697 wouldn't apply to mobile, since it would be support for per-plugin prefs, and we only have one plugin on mobile right now)
The part of bug 549697 that we depended on is now split off into bug 707886.
I'm working on a patch for this.
*** Bug 709425 has been marked as a duplicate of this bug. ***
Created attachment 580994 [details] [diff] [review]
This works, but after changing the pref, I'm seeing the integer pref value appear below "Enable Flash" in the main preference UI. When I come back to the Preferences interface, it's gone, so it only happens right after editing the value. I'll attach a screenshot to show what's happening.
Also, I ended up using a string type for this fake "plugin.enable" pref because that's what made the UI work for me, but maybe there's a way to get it to work using an integer type?
Created attachment 580996 [details]
See the "1" under Enable Flash? That appears after I change the value, but I don't want it to ever be there.
Oh, I see, it's because we do this .setSummary() when the pref changes:
Since this is going to be the only ListPreference, I think we could just get rid of this line. Even if we wanted that summary to appear, we should probably set it up when we initialize the pref UI, not just after a pref changes.
Created attachment 581000 [details] [diff] [review]
This patch does what I suggest in comment 11 :)
Comment on attachment 581000 [details] [diff] [review]
* Can we move setPluginPreference/getPluginPreference to the PluginHelper? I don't like adding more and more to BrowserApp if possible.
* We really do need the List Summary. Can we map the values to the display string?
r- for the list summary bit
Created attachment 581055 [details] [diff] [review]
Getting the string for the pref entry in onPreferenceChange is kinda gross, but that's the only way I found to get the entry that corresponds to newValue (we can't use .getEntry() because the pref hasn't changed yet when the event is fired).
Comment on attachment 581055 [details] [diff] [review]
Looks fine to me. I just wish we had better adapter logic for Fennec prefs -> UI prefs; it seems like every preference we add is a special case that requires more code fudging.
Now all we need is bug 707886 to land, and this feature should be pretty good to go (modulo any bugs that pop up).
Is this going to be on the desktop or is mobile only ? If its mobile only, is there a separate bug for desktop ?
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #17)
> Is this going to be on the desktop or is mobile only ? If its mobile
> only, is there a separate bug for desktop ?
Right now it is only a mobile feature, but the platform work helps with the desktop feature. The desktop work is being tracked in bug 549697.
Verified fixed on:
Firefox 13.0a1 (2012-03-02)
Device: Samsung Galaxy S2
OS: Android 2.3.4
Testcase added under BFT - options (preferences):