If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Fix what preference "Activate all plugins by default" in SeaMonkey preferences window sets

RESOLVED FIXED in seamonkey2.23

Status

SeaMonkey
Preferences
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mcsmurf, Assigned: mcsmurf)

Tracking

Trunk
seamonkey2.23

SeaMonkey Tracking Flags

(seamonkey2.20 wontfix, seamonkey2.21 fixed, seamonkey2.22 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
The preference "Activate all plugins by default" in SeaMonkey preferences (Advanced->Script&Plugins) currently sets plugins.click_to_play to true. That's not correct anymore, the relevant pref is now plugin.default.state. plugin.default.state=0 means "deactivate all plugin", 1 means click-to-play, 2 means activate all plugins. This preference reflects to the GUI when looking at about:addons -> Plugins. The plugins.click_to_play pref just displays the "Ask to activate" UI in about:addons. It does not CTP (de)activate any plugin (see Bug 866390).
(Assignee)

Comment 1

4 years ago
Bug 549697 changed the pref, so currently all release trains are affected. Marking wontfix for SeaMonkey 2.20, too late to fix it.
status-seamonkey2.20: --- → wontfix
status-seamonkey2.21: --- → affected
status-seamonkey2.22: --- → affected
status-seamonkey2.23: --- → affected
(Assignee)

Updated

4 years ago
Duplicate of this bug: 903685
(Assignee)

Comment 3

4 years ago
Neil: Do you know where in the SeaMonkey code I could place migration code for a pref? I need to check once if plugins.click_to_play is set to true and based on that set another pref (plugin.default.state). This can happen after all the UI from browser startup has been displayed, so this does not necessarily need to happen during startup.
Flags: needinfo?(neil)

Comment 4

4 years ago
Frank:
Firefox puts this sort of migration code in their nsBrowserGlue.js

Comment 5

4 years ago
Interesting, the Add-ons Manager has a bug whereby setting the default state to 1 only displays properly if plugins.click_to_play is true...

(In reply to Frank Wein from comment #3)
> Neil: Do you know where in the SeaMonkey code I could place migration code
> for a pref? I need to check once if plugins.click_to_play is set to true and
> based on that set another pref (plugin.default.state). This can happen after
> all the UI from browser startup has been displayed, so this does not
> necessarily need to happen during startup.

So, would _updatePrefs in nsSuiteGlue.js be suitable?
Flags: needinfo?(neil)

Comment 6

4 years ago
See https://bugzilla.mozilla.org/show_bug.cgi?id=866390 - the behaviour is inhereted from Firefox which does exactly the same thing wrong.
The last post says: "The actual click-to-play changes (aside from block-listing) will come in Firefox 24 and will have visible UI and sensible default states for plugins."
So, maybe, instead of breaking our brains, it may already fixed in the v24 alpha/beta tree? Seems like they want to include a domain based whitelist/blacklist for the plugins to be enabled or click-to-play.
(Assignee)

Comment 7

4 years ago
Firefox 24 has a new, bit different UI regarding how enabling/blocking plugin works. When you click on a click-to-play blocked plugin there, a doorhanger UI (that's the GUI that opens when click the plugin icon in the URL bar) opens where you can choose to enable/block this plugin type and remember that decision for this page. But if you're interested in that, I suggest you download a current Firefox beta build :).
The problem with migrating plugins.click_to_play pref probably cannot be solved though. The problem is the core code changed the meaning of the plugins.click_to_play pref. These days setting plugins.click_to_play to true basically means: Activate the click-to-play UI in the about:addons plugins UI. But in our code we don't know from what version you updated from, though the application could make some guesses (but in a sane way I think).
(Assignee)

Comment 8

4 years ago
Created attachment 792824 [details] [diff] [review]
Patch

Neil: This patch fixes the pref window and also enables the CTP UI under about:addons by setting click_to_play to true (I see no reason why we should hide the CTP feature there). This patch is not quite accurate as it ignores the difference between 0 and 1 for plugin.default.state (though 0 can only happen when a user manually sets that pref). The checkbox only gets checked when the pref has the "enable all plugins" state. This patch is branch-safe as it does not change any strings. Not sure if we should handle the "0" pref value separately. I don't think many users will use that, after all there is still the plugins.disabled pref.
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
Attachment #792824 - Flags: review?(neil)

Comment 9

4 years ago
Comment on attachment 792824 [details] [diff] [review]
Patch

So, on trunk, should we follow up by changing to something that will cope with a value of 0 too?

>+                onsynctopreference="return document.getElementById('scripts_pane').WritePluginDefaultState()"/>
Note: you can just write "return this.checked ? 2 : 1;" here. You have various choices for onsyncfrompreferences, such as "return document.getElementById(this.getAttribute('preference')).value == 2;".
Comment on attachment 792824 [details] [diff] [review]
Patch

r=me with above nits fixed.
Attachment #792824 - Flags: review?(neil) → review+
(Assignee)

Comment 11

4 years ago
Created attachment 792866 [details] [diff] [review]
Patch for checkin

Ah, good ideas to shorten the code like that.

Yes, for trunk I have additional ideas in mind. But there probably the "plugins.disable" pref then needs to be removed from the GUI. Explaining the difference to the user between plugins.disable = true and plugins.default.state=0 is not worth it.
Attachment #792824 - Attachment is obsolete: true
Attachment #792866 - Flags: review+
(Assignee)

Comment 12

4 years ago
Pushed: https://hg.mozilla.org/comm-central/rev/f50955f8a0c3
status-seamonkey2.23: affected → fixed
Target Milestone: --- → seamonkey2.23
(Assignee)

Comment 13

4 years ago
Comment on attachment 792866 [details] [diff] [review]
Patch for checkin

[Approval Request Comment]
Regression caused by (bug #): Bug 866390
User impact if declined: Preference in pref window does not behave like it should
Testing completed (on m-c, etc.): Works fine on mozilla-central
Risk to taking this patch (and alternatives if risky): Small patch, very low risk, changes some XUL code only.
String changes made by this patch: -
Attachment #792866 - Flags: approval-comm-beta?
Attachment #792866 - Flags: approval-comm-aurora?

Updated

4 years ago
Attachment #792866 - Flags: approval-comm-beta?
Attachment #792866 - Flags: approval-comm-beta+
Attachment #792866 - Flags: approval-comm-aurora?
Attachment #792866 - Flags: approval-comm-aurora+
(Assignee)

Comment 14

4 years ago
Pushed to aurora: https://hg.mozilla.org/releases/comm-aurora/rev/839a532b06b8
Pushed to beta: https://hg.mozilla.org/releases/comm-beta/rev/6513e29c9aff

=> FIXED, I'll open a new bug for better UI in prefs

Joachim: Unfortunately we could not migrate the old click_to_play pref, but at least we've fixed the UI now. The problem is Mozilla devs decided to change the meaning of that preference which is usually not a good thing to do. This makes pref migration basically too difficult.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-seamonkey2.21: affected → fixed
status-seamonkey2.22: affected → fixed
status-seamonkey2.23: fixed → ---
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.