84227 - System Preference panel is not lockable

Status: VERIFIED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Rodney Velasco]

Elements in the Advaced -> System Preference panel are not lockable.  eddyk
implemented a locking scheme to lock elements in the General Preference panel. 
Info on his work can be found on this newsgroup posting
news://news.mozilla.org/3AC8F5D2.6CA02623%40netscape.com .

The pref-winhooks.xul implements some elements that modify the system registry.
 eClient requires that all user-editable elements in the preference dialog be
disabled if a preference is locked.

While these elements are not mapped to preferences, we still need them to be
locked.  Probably, adding a set of parallel preferences that map to the registry
entries will fufill that requirement for disabling.

Comment 1

[Rodney Velasco]

assigning myself to QA contact...adding sarah to Cc.

Comment 2

[Viswanath Ramachandran]

-> eddyk, by request. thanks for taking this!!

Comment 3

[eddyk]

Since the 2 sub-windows with the checkboxes get their data from the registry and
not the pref system, there are 2 options I am looking at:

1.  create a new pref for each 6 file types, 5 protocols.

2.  Create 1 pref for each group (files and protocols) to lock all checkboxes in
them.

I'm leaning towards 2, not because it's marginally easier (it's not), but for
future maintenance.  Newly added file types and protocols would require
additional work that will probably be forgotten.  option 2 allows us to keep
locking working at a larger grained level with the benefit of lower maintenance.

If anyone has any objections to this approach and would prefer to see #1, let me
know ASAP.

Comment 4

[eddyk]

Erg.  After some more investigation I'm going with option 1.

The pref strings to lock the elements will be somewhat dynamic.  IOW, they will
be generated on the fly, following a convention.  They won't be listed in the
xul or js like in other panels.

Comment 5

[eddyk]

Attachment: patch to lock system panel

Comment 6

[eddyk]

Quick description of fix:

There is a loop that goes through a list of "settings" and retrieves those
settings and values from the registry.

I've added to the loop an additional check for preferences with the same name as
those settings.  If those prefs are locked, the ui will be locked as well.

The prefs all start with the string: system.windows.lock_ui.

The current list of settings are:
isHandlingHTML
isHandlingJPEG
isHandlingGIF
isHandlingPNG
isHandlingXML
isHandlingXUL
isHandlingHTTP
isHandlingHTTPS
isHandlingFTP
isHandlingCHROME
isHandlingGOPHER
showDialog

Any new settings that are created and added to the dialog should be
automatically locked if the pref is present and locked.

The trick is documenting this so that current and future settings are easily
available to people who need to know (IS admins, people working on eClient tools).

Comment 7

[Srilatha Moturi]

The fix looks good except that it would be good to have try catch block around 
the following line of code, if getBranch throws exception.
prefBranch = prefService.getBranch(prefbase);
Eddy, find out from brian if getBranch throws an exception or not.

If the above issue is addressed r=srilatha

Comment 8

[eddyk]

As explained by bnesse:

It is not necessary to catch it.  Even if the branch throws an error, it is
generally better to let the exception go out because catching doesn't let it
bubble up.

Comment 9

[Blake Ross]

+var prefService = null;
+var prefBranch = null;

Prefix these with "g" to indicate globalness.

+    var prefbase = "system.windows.lock_ui.";

This should be const.

+        if (prefBranch.prefIsLocked(setting)) {

Null-check prefBranch first?

+            checkbox.setAttribute( "disabled", "true" );

Use checkbox.disabled = true;

With that, sr=blake

Comment 10

[eddyk]

checked in.

Comment 11

[Rodney Velasco]

Christine Wozniak, these are Windows specific preferences that should be
documented for eClient team.  Added you to Cc to keep you in the loop.

system.windows.lock_ui.isHandlingHTML
system.windows.lock_ui.isHandlingJPEG
system.windows.lock_ui.isHandlingGIF
system.windows.lock_ui.isHandlingPNG
system.windows.lock_ui.isHandlingXML
system.windows.lock_ui.isHandlingXUL
system.windows.lock_ui.isHandlingHTTP
system.windows.lock_ui.isHandlingHTTPS
system.windows.lock_ui.isHandlingFTP
system.windows.lock_ui.isHandlingCHROME
system.windows.lock_ui.isHandlingGOPHER
system.windows.lock_ui.showDialog

Comment 12

[eddyk]

Note that the type of these prefs are not important.  The code only checks to
see if they are locked, it doesn't check the value of the prefs.

Comment 13

[Rodney Velasco]

verified on windows trunk build 2001071704.  Since this is windows specific,
will not verify on other tier 1 platform.

Comment 14

[Rodney Velasco]

Attachment: Hashed CFG file to lock system pref panel ites

Comment 15

[Rodney Velasco]

To test and activate CFG, save attachement as "netscape.cfg".  Edit the
all-ns.js (or if using mozilla all.js file) and add the following lines to
activate the CFG.

pref("general.config.filename", "netscape.cfg");
pref("general.config.vendor", "netscape");

Comment 16

[eddyk]

In discussing another unrelated bug with someone, a question was brought up in
my mind:

With the current patch, the ui can be locked, but is it desirable to have the
value of these registry items set from the prefs as well?  Currently, admins
will not be able to set the value of the checkboxes, only that they be locked.

Comment 17

[Chip Clark]

Yes - there will be those that want to set the value and lock it.

Comment 18

[Hugo Kortschak]

The statement 'eClient requires that all user-editable elements in the
preference dialog be disabled if a preference is locked.' in the description of
this bug should not be the approach to be chosen. 
Assigning a preference a value should be fully independent to its management;
set as default only or set locked (defaultPref ¦ lockPref command).
For example, during a migration from Communicator to eClient 6/7 it is
interesting to switch the 'default browser settings' for an enterprise on a
certain milestone. This could be done fully with autoconfig by setting the
preferences within the resp. autoconfig-files for Communicator and eClient. 
Thus, I support the comments #16 and #17.