Bug 70033 handles the general pref locking by updating the nsWidgetStateManager but the account settings uses a different mechanism and needs to also be updated to be locked-pref aware.

shouldn't this go to mailnews fe...?

I do believe you're correct. Trying to figure out how to change it to the correct component...

-> mailnews fe, i do believe. thx, eddyk!

cc: hong. Please reassign to your QA who is working on locked prefs.

Adding myself to the cc list.

lets do this in 0.9, not at the last minute in 0.8.1

reassigning qa to rvelasco@netscape.com.

The first attachment is an ugly, ugly hack that I just want to put up for reference. I don't plan on checking this in unless nothing else works. While it may work, it's non-maintainable. The mechanism for disabling is fine. It's the way I generate the preference strings in this approach that bothers me. In increasing level of goodness (and difficulty), I'm going to look at the following approaches: 1) extend the idea of the first patch, but instead of generating the pref strings in the xul code, query the appropriate object for the id -> prefstring mapping. This would be more maintainable and less error-prone, IMO. 2) instead of generating prefstrings on the fly in various objects, put the prefstrings in the xul elements, much is as done currently in the main preferences window. This will require some type of keyword substitution, and possibly other issues will come into play.

this is an incredible hack that is going to be very difficult to maintain as we add and remove new attributes on the servers, identities, etc.

The second attachment is a different approach, which is hopefully a step in the right direction of the eventual goal to fully integrate with the general pref FE code. Please comment on this before I go too far with it. :) Thanks

suggestion: don't create cryptic strings that must be broken up, when you have the option of breaking them up from the very beginning i.e. instead of xpref="type=string,prefstr=mail.identity.%identitykey.organization" how about preftype="string" prefobjtype="identity" prefpostfix="organization" then you can look at each individual string in there with .getAttribute(), and reconstruct the preferences appropriately, and you can treat each string as an opaque value. we have a parser, it's called xpat, and it parses xml quite nicely - take advantage of that! :)

Sorry about that, that's my fault. I proposed that we investigate alternate ways of doing this instead of adding more and more pref* attributes. I'm getting tired of seeing: <textfield pref="true" prefattribute="value" prefstring="foobar.foo_bar" preftype="string" .../> Not to mention that it suggests that no relationship exists between the attributes (but it does -- they're basically dependent on one another) since they're all top-level, independent attr.'s But seeing this comma-delimited list in action is disappointing, also...I didn't realize it would look so confusing. I'd still like to investigate other ideas, but that's not really within the scope of this bug. Sorry for bringing it up, Eddy!

Thanks for the feedback guys. Alec, do you have any reservations about the %token within the prefstring? Right now, I feel that would be clearer and more flexible than pre/post-fix attributes. More than one token could be substituted, and it's easier to see at a glance where the variable(s) is placed. I agree on the separate attributes for type, value, etc.

I guess I take issue with Yet Another Token Substitution method - there are a million all over the codebase. I'm all for flexibility, but the way I designed the account manager, no pref will ever have multiple keys in it... i.e. you'll never have mail.%server%.%identity%.foo or anything like that, and in fact to do so would generate dependancies between the objects that don't already exist, which would be bad. if you must do token substituion, please at least do %varname% (double-%) because that's what people are using (unfortunately) in the string bundles.

Sorry to hoist YATS (tm) on the product. At this point, I feel it's the best solution. It fits in better with the exiting pref usage, which I hope to integrate with later. LDAP and other pref users may need multiple substitution. I wasn't aware of string bundles, and am looking at it for ideas and possible areas where I might be able to share code/ideas. Thanks again,

The 4/29 diff shows the implementation of locking using the same pref attributes as used in the main pref dialog, with the exception of the %token% strings. We're not using WSM and nsPrefWindow yet, but this should simplify the transition. This isn't complete yet, but I'd like to get some feedback if possible. I still need to lock the (new|delete|default) account buttons on the left side. The new account locking will require some fiddling with the main menu code. It seems like a good use for 'observes'. I'm not sure how likely it is for full WSM and nsPref integration before the final freeze. If it turns out it can't make it in time, would a partial implementation such as this be totally unacceptable?

I'm warming to this approach after seeing the patch - it's at least consistent with the nsPrefWindow stuff, as you said.

First: I've opened a new bug 79305 to handle the second portion of this task, which is to use the same code as the general prefs dialog. I'd like to check in this code, which can be summarised as the bare essentials to get fe locking required by enterprise users. This second patch I'll be adding locks the account button on the left side of the AM dialog. It alos disables the new account function available from the account central page and from the File|New|Account menu item.

I'd like to clarify why I'm looking to check this in ASAP. 5/11 is a code freeze deadline I'd like to make. Now I need review and sr.

it seems like you should be looking at the locked status of the existing pref "mail.accountmanager.accounts" to determine if add/delete should be enabled or disabled...

can I get sr= and r= on this patch + new file?

asked racham for r= and alecf for sr =

patch looks ok, two requests: 1) add even more comments, this stuff is confusing 2) can you use nsIPrefBranch instead of the deprecated nsIPref?

also, some of your indenting in the JS is wacky (the XUL looks fine) - it would take more time for me to paste the places where it's wacky then it would for you just look over the patch to see..

r=racham.

Setting target milestone to 0.9.2 (check it in anytime, even before, when the tree is open for). Per PDT triage.

cool thanks.. sr=alecf

marking fixed. for those interested, bug 79305 will be continuing this work.

All mail/news account prefs are lockable using eddyk's test case to lock prefstrings. In order to lock the New Account Button you will need to specify this lockPref function in the all-ns.js file or a hashed configuration file: lockPref("mail.accountmanager.accounts", true); Offline elements in the mail/news window is not lockable yet. Filed separate bug for that issue bug 82805. I'm going to leave this as resolved fix until bug 82805 is resolved. adding dianesun to Cc.

Offline prefs section is now lockable, but there are other elements in Mail/News account settings that need to honor locked prefs. Still awaiting on bug 85335 before I can verify this bug. I'll be filing more locking bugs since GUI has changed since I filed this issue. Will add the bugs as dependencies once filed.

bug 85335 has been verified...this bug is too now verified due to dependency on that bug. Marking verified.