289136 - Prefs dialog landing breaks preference locking

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

It looks like the prefs dialog landing added UI to about:config to unlock locked
prefs.  That rather defeats the purpose of having pref locking in the first
place; we should either remove the about:config UI or remove preference locking
altogether (and tell corporates who want it to enforce I/S policies "tough").

Comment 1

[Matthias Versen [:Matti]]

*** Bug 289128 has been marked as a duplicate of this bug. ***

Comment 2

[Mike Kaply [:mkaply]]

Attachment: Fix for problem

Remove the lock/unlock stuff

Comment 3

[Mike Kaply [:mkaply]]

We need to make sure that the ability to unlock prefs is not in an official release.

Comment 4

[Mike Connor [:mconnor]]

Comment on attachment 179715 [details] [diff] [review]
Fix for problem

This isn't the right way to address this, at leat not for 1.5.	How about a
pref to disable the menuitems, and that can be locked?	That doesn't have an
l10n impact, and lets developers continue to have a useful feature for testing
prefpanels.

Comment 5

[Mike Kaply [:mkaply]]

This patch doesn't have to have l10n impact - I can just not change the DTD.

As far as having a pref to remove locking of prefs - that just makes extra work
for people trying to lock pref items.

If we need this ability for developers, then it should be debug only or in some
developer overlay.

Comment 6

[Asa Dotzler [:asa]]

developer convenience is no longer a serious concern, we should re-evaluate the
patch.

Comment 7

[Mike Connor [:mconnor]]

I'm starting to think that what we need to have is some sort of
MOZ_OFFICIAL_RELEASE bit that we can use in the preprocessor so we can have
things in nightlies that aren't in release builds.  This is a good example, a
random chunk of topsite testing as a link in a QA menu would be another.  I'm
fine with killing this on branch only, and leaving this open for a more balanced
fix for the trunk.

Comment 8

[Benjamin Smedberg]

Asa, why did you remove the blocking flag? This is a serious issue for corporate
deployment.

Comment 9

[Asa Dotzler [:asa]]

sorry. I misread mconnor's comment and thought he said this was fixed on the
branch. we definitely want this.

Comment 10

[Mike Connor [:mconnor]]

Comment on attachment 179715 [details] [diff] [review]
Fix for problem

r=me on simply removing the menuitems (no l10n impact) on branch only.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Might be a good idea to leave the functions there too, to make it easy for an
extension to re-enable this.

Comment 12

[Mike Connor [:mconnor]]

which is why I specified removing the menuitems only.

Comment 13

[:Gavin Sharp [email: [e-mail]]]

Right, I just wanted to clarify, since there was mention of not touching the
DTD, but no mention of not touching the JS file.

Comment 14

[Mark Sammons]

(In reply to comment #12)
> which is why I specified removing the menuitems only.

This might be outwith the scope of this bug, but maybe a good idea would be to 
have some sort of an unlockable Master Lock so that once triggered, any locked 
settings are perma-locked for the session.

If an administrator sets up Firefox with a set of locked variables, then they 
do not want these to be overridden by an extension which sets different locked 
settings or re-enables this "Unlock" UI.

Comment 15

[:Gavin Sharp [email: [e-mail]]]

Attachment: remove menuitems (1.8 branch only)

Comment 16

[Mike Schroepfer]

Comment on attachment 196719 [details] [diff] [review]
remove menuitems (1.8 branch only)

Per bug meeting - approved for 1.8b5.

Comment 17

[:Gavin Sharp [email: [e-mail]]]

1.8 Branch:
mozilla/toolkit/components/viewconfig/content/config.js; new revision: 1.12.2.2;
mozilla/toolkit/components/viewconfig/content/config.xul; new revision: 1.5.8.1;

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

shouldn't this land on the trunk too?

Comment 19

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #18)
> shouldn't this land on the trunk too?

see comment 7

Comment 20

[Boris Zbarsky [:bzbarsky]]

Well, we need to fix this by FF3.  Too bad we have no flag for that.

Comment 21

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs,
filter on "beltznerLovesGoats" to get rid of this mass change

Comment 22

[Phil Ringnalda (:philor)]

Personally, bug 352032 made me think the number of developers needing this to test pref locking approaches zero.

Comment 23

[Mike Kaply [:mkaply]]

Phil(In reply to comment #22)
> Personally, bug 352032 made me think the number of developers needing this to
> test pref locking approaches zero.
> 

But wouldn't having this feature have made your testing of bug 352032 much easier?

Comment 24

[Phil Ringnalda (:philor)]

Knowing that I was testing something that didn't work the way a release would, but still not entirely trusting that it wouldn't muck something up? No. I wouldn't check it in without testing it the way we release (other than by forgetting that I'd have to change something to make it like a release, which I would have done if I didn't start out fixing it for 1.8.1), and once I'd spent the five minutes creating a mozilla.cfg, they were spent.

I'd say it's only useful to people doing a complete rewrite of the whole prefs dialog, of which we have two, one still active. How about it, Jeff - did you find that you needed to toggle prefs from locked to unlocked to locked to unlocked while doing your rewrite?

Comment 25

[:Gavin Sharp [email: [e-mail]]]

Attachment: completely remove the items

I agree with Phil. Per discussion with mconnor we're just going to remove these completely on the trunk (and for Firefox 3).

Comment 26

[:Gavin Sharp [email: [e-mail]]]

mozilla/toolkit/components/viewconfig/content/config.js 	1.22
mozilla/toolkit/components/viewconfig/content/config.xul 	1.12
mozilla/toolkit/locales/en-US/chrome/global/config.dtd 	1.10

Comment 27

[Jeff Walden [:Waldo]]

(In reply to comment #24)
> How about it, Jeff - did you find that you needed to toggle prefs from locked
> to unlocked to locked to unlocked while doing your rewrite?

For the record, no.  The preference binding handles this automagically, I believe; also, I was far less concerned about preference locking than I was about just getting the functionality right, so I didn't put that much effort into making sure locked prefs worked correctly.