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

Need to disable UI when they are locked

VERIFIED FIXED in mozilla1.0.1

Status

SeaMonkey
MailNews: Account Configuration
P1
normal
VERIFIED FIXED
16 years ago
13 years ago

People

(Reporter: tao, Assigned: racham)

Tracking

Trunk
mozilla1.0.1

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt2 rtm],custrtm+,[ETA 06/24])

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

16 years ago
When email/news account preferences are locked, the associated GUI should be
greyed out should users won't be able to modify them.
(Reporter)

Updated

16 years ago
Blocks: 144547
(Reporter)

Updated

16 years ago
Blocks: 141352
(Reporter)

Updated

16 years ago
No longer blocks: 144547
(Reporter)

Comment 1

16 years ago
nominating
Keywords: nsbeta1

Updated

16 years ago
Keywords: nsbeta1 → nsbeta1+
Whiteboard: [adt2 rtm]
Target Milestone: --- → mozilla1.0.1

Comment 2

16 years ago
QA over to me.  customization issue.
QA Contact: nbaca → rvelasco
(Reporter)

Updated

16 years ago
Whiteboard: [adt2 rtm] → [adt2 rtm],custrtm+
(Assignee)

Comment 3

16 years ago
Created attachment 86176 [details] [diff] [review]
patch, v1.0
(Assignee)

Comment 4

16 years ago
Created attachment 86177 [details] [diff] [review]
ns (commercial tree) patch, v1.0
(Assignee)

Comment 5

16 years ago
Created attachment 86178 [details]
sample file used in locking a preconfigured account

This file contains pre-configured settings in all.js and more in mn_prefs.txt
(which I used to generate a cfg file using make_cfg tool). In mn_prefs.txt, I
have added comments next to each pref (from all those AccountManager panels)
about the pref and the values we can set in case of certain prefs.

For locking Addressing panel prefs, use following prefs.

mail.identity.<idKey>.overrideGlobal_Pref  this is a boolean pref
mail.identity.<idKey>.directoryServer this is a string pref and the value is
"ldap_2.servers.<server-name>"
(Assignee)

Comment 6

16 years ago
I have requested Rodney & Ninoschka to run their test suites with the optimized
build I have provided them.

Also, I have requested Srilatha and Seth to take a look at the patch and provide
any feedback.
Status: NEW → ASSIGNED
looks good.  some minor comments / questions:

1)

Trying to understand...instead of disabling / enabling the ui using the
prefstring trick, we've got another pref.

-  if (nsPrefBranch.prefIsLocked(addAccountButton.getAttribute("prefstring")))
+  if (!nsPrefBranch.getBoolPref("mail.enable_new_account_addition"))
     canCreate = false;
-  //if (nsPrefBranch.prefIsLocked(duplicateButton.getAttribute("prefstring")))
-  //  canDuplicate = false;
-  if (nsPrefBranch.prefIsLocked(setDefaultButton.getAttribute("prefstring")))
+  if (!nsPrefBranch.getBoolPref("mail.enable_default_account_setting"))
     canSetDefault = false;
-  if (nsPrefBranch.prefIsLocked(removeButton.getAttribute("prefstring")))
+  if (!nsPrefBranch.getBoolPref("mail.enable_account_deletion"))
     canDelete = false;
 

-      <button label="&addAccountButton.label;" oncommand="onAddAccount(event);"
id="addAccountButton"
-              prefstring="mail.accountmanager.accounts"/>
+      <button label="&addAccountButton.label;" oncommand="onAddAccount(event);"
id="addAccountButton"/>
       <button label="&setDefaultButton.label;" oncommand="onSetDefault(event);"
disabled="true" id="setDefaultButton"
               prefstring="mail.disable_button.set_default_account"/>

Since you removed these three lines,

-  if (nsPrefBranch.prefIsLocked(addAccountButton.getAttribute("prefstring")))
-  if (nsPrefBranch.prefIsLocked(setDefaultButton.getAttribute("prefstring")))
-  if (nsPrefBranch.prefIsLocked(removeButton.getAttribute("prefstring")))

you should have removed three prefstring attributes from AccountManager.xul,
right?  I think you missed two.

2)

Does this mean if the fcc is locked, we aren't going to set it up correctly, or
am I misreading the code?

You'd think we'd want to set it up, and then disable it.

 // Check the Fcc Self item and setup associated picker state 
 function setupFccItems()
 { 
+  if (!gFccRadioElemChoiceLocked) {
     var broadcaster = document.getElementById("broadcaster_doFcc");
 
     var checked = document.getElementById("identity.doFcc").checked;
@@ -313,6 +315,7 @@
 	}
     else
         broadcaster.setAttribute("disabled", "true");
+  }
 }

3)

did you just choose another pref, to keep pref names uniform?  Does this require
a change to CCK?
 
-      { prefstring:"disable_button.selectFolder", id:"selectFolderButton"}
+      { prefstring:"selectFolderButton", id:"selectFolderButton"}
     ];
 
4)
 
+      { prefstring:"select_custom_prefs", id:"identity.select_custom_prefs"},
+      { prefstring:"select_global_prefs", id:"identity.select_global_prefs"},

and

       <radiogroup id="identity.use_custom_prefs" wsm_persist="true"
genericattr="true" 
                   preftype="bool"
prefstring="mail.identity.%identitykey%.use_custom_prefs" 
                   oncommand="EnableDisableCustomSettings();">
-        <radio value="false" label="&useGlobalPrefs.label;"/>
-        <radio value="true" label="&useCustomPrefs.label;"/> 
+        <radio id="identity.select_global_prefs" value="false"
label="&useGlobalPrefs.label;"/>
+        <radio id="identity.select_custom_prefs" value="true"
label="&useCustomPrefs.label;"/> 
       </radiogroup>
     </hbox>
 
it looks like the pref string is already there
(mail.identity.%identitykey%.use_custom_prefs) to lock the radiogroup.
was it not working?  It now looks like you are locking the individual radio
elements.  It seems like locking the radiogroup would be enough.

5)

 function CreateWebMailAccount()
 {
     try {
+      if (!prefs.getBoolPref("mail.enable_new_account_addition"))
+        return null;

While this is creating a new account, do we want an entire new pref for this?
(Assignee)

Comment 8

16 years ago
Seth,

Thanks for the reviews.

* Comment 1 : Though this works, I guess I can get back to prefstrings mechanism
as that eliminates need for adding new default prefs.

* Comment 2 : It works. But, bit round about. SetupFccPickerState() can be
modified to achieve the control we want in a better way. I will remove that
check locking in there. Thanks.

* Comment 3 : While doing locking, I have noticed for buttons, value of true or
false didn't matter. So, I thought it is just fine to have a pref that suggest
that we are locking without any verbs like diable and enable. I will check other
such instances also.

* Comment 4 : This particular guess, under use custom prefs choice, we have a
whole new set of item (when user selects custom receipts). If we simply lock
everything based on radio group lock, we would take away the flexibility of
locking subcomponents under the custom receipts item. With the current patch,
one can lock elements of choice under custom receipts choice.

* Comment 5 : I have to check for this pref here as we need to block any account
creation activity here.

thanks,
Bhuvan
(Assignee)

Updated

16 years ago
Whiteboard: [adt2 rtm],custrtm+ → [adt2 rtm],custrtm+,[ETA 06/15]
(Assignee)

Updated

16 years ago
Priority: -- → P1
(Assignee)

Comment 9

16 years ago
Created attachment 87907 [details] [diff] [review]
patch, v1.1
Attachment #86176 - Attachment is obsolete: true
(Assignee)

Comment 10

16 years ago
Created attachment 87908 [details] [diff] [review]
ns (commercial tree) patch, v1.1
Attachment #86177 - Attachment is obsolete: true
(Assignee)

Comment 11

16 years ago
Created attachment 87909 [details]
new sample pre-config file

This file has few more new prefs to be part of global prefs file and also some
new prefs to be locked in order to achieve locking without introducing new
prefs.
Attachment #86178 - Attachment is obsolete: true

Comment 12

16 years ago
Comment on attachment 87907 [details] [diff] [review]
patch, v1.1

sr=bienvenu
Attachment #87907 - Flags: superreview+

Comment 13

16 years ago
Comment on attachment 87908 [details] [diff] [review]
ns (commercial tree) patch, v1.1

sr=bienvenu
Attachment #87908 - Flags: superreview+

Comment 14

16 years ago
Using bhuvans second optimized build I was having some problems with with the
security preferences:

lockPref("mail.identity.id9991.signingCertSelectButton", false);          
lockPref("mail.identity.id9991.encryptionCertSelectButton", true);       

The radio buttons are locked down from user interaction at first launch, but the
text area field to specify a valid certificate unlocks the check box and radio
box if you enter in random text in that field, exit the mail news account
settings pref panel, and open it up again. If we lock down the two above
preferences we should lock down the text area field as well. 

All other prefs are locked down as expected.  No other unexpected behavior seen
with the rest of the prefs defined in the attached sample pre-config file.
(Assignee)

Comment 15

16 years ago
Those text areas fields (Security panel) not editable. Cert names are filled in
those boxes only after user clicks on the button and selects a cert. Once we
lock those buttons that panel is pretty much locked. Anyway, you can list the
steps that led you to achieve the unlocking. Thanks.

Comment 16

16 years ago
Comment on attachment 87908 [details] [diff] [review]
ns (commercial tree) patch, v1.1

r=srilatha
Attachment #87908 - Flags: review+

Comment 17

16 years ago
Comment on attachment 87907 [details] [diff] [review]
patch, v1.1

r=srilatha
Attachment #87907 - Flags: review+
(Assignee)

Comment 18

16 years ago
Fix checked in on the trunk. Marking Fixed. Thanks for reviews and comments.

Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Keywords: adt1.0.1, mozilla1.0.1
Resolution: --- → FIXED

Comment 19

16 years ago
Great work bhuvan, all mail/news preferences defined in the latest trunk builds
are lockable.  

A note to code reveiewers, since bhuvan locked all the prefs on a per pref basis
it would be nice for any new preferences added to the mail/news account settings
UI follow the locking guidelines as defined in this newsgroup posting:

news://news.mozilla.org/3AC8F5D2.6CA02623%40netscape.com

I'm not aware of anymore preferences being added to the mail/news account
settings this late in the game, but any future work on prefs should be lockable
from a customization point of view (unless bug 79305 ever gets fixed), or else
I'll be filing bugs to try and get them locked.  Thanks again bhuvan for fixing
this for the customization group.

Verified on Trunk
Macos X.1.3 2002061908
Macos9.2.2 2002061908
Linux 2.4.7-10 2002061908
Windows NT 4.0 2002061908
Status: RESOLVED → VERIFIED

Updated

16 years ago
Whiteboard: [adt2 rtm],custrtm+,[ETA 06/15] → [adt2 rtm],custrtm+,[ETA 06/24]

Comment 20

16 years ago
Adding adt1.0.1+ on behalf of the adt for checkin to the 1.0 branch.  Please get
drivers approval before checking in. When you check this into the branch, please
change the mozilla1.0.1+ keyword to fixed1.0.1
Keywords: adt1.0.1 → adt1.0.1+

Updated

16 years ago
Attachment #87907 - Flags: approval+

Comment 21

16 years ago
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Keywords: mozilla1.0.1 → mozilla1.0.1+
(Assignee)

Comment 22

16 years ago
Thanks for the approvals. Will check this in as soon as branch opens.
(Assignee)

Comment 23

16 years ago
Fix checked in on the branch. Files (all right ones) got commited accidentally 
without checkin comments. So, I am providing comments for the record here 
(already added to the comment box on the tinderbox).

Fixing bug 144563. Disable mailnews AccountManager panel elements when the 
corresponding preferences are locked. This is a useful feature for 
Administrators and vendors. Special configuration tool is need to be used for 
generating a config file to feed the locked prefs to the app. r=srilatha, 
sr=bienvenu, a=drivers,adt.

Adding fixed1.0.1 keyword.
Keywords: mozilla1.0.1+ → fixed1.0.1

Comment 24

16 years ago
verified on commercial branch

Linux 2.4.7-10 2002062806
Windows NT 4.0 2002062808
Mac OS X.1 2002062809
Mac OS 9.2.2 2002062805
Keywords: fixed1.0.1 → verified1.0.1
(Assignee)

Comment 25

15 years ago
*** Bug 96152 has been marked as a duplicate of this bug. ***
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.