Closed Bug 82805 Opened 24 years ago Closed 24 years ago

Offline and Disk Space Prefs should be lockable.

Categories

(SeaMonkey :: MailNews: Message Display, defect, P2)

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.2

People

(Reporter: rvelasco, Assigned: dianesun)

References

()

Details

(Whiteboard: [PDT+] Request R&SR)

Attachments

(11 files)

User Prefs "mail.server.server1.offline_download" "mail.server.server1.download_bodies_on_get_new_mail" need to be lockable according to eddyk's locking spec. Information regarding how to lock elements in Pref window can be found here... news://news.mozilla.org/3AC8F5D2.6CA02623%40netscape.com There are no prefstrings available for select box and Disk Space pref elements, those elements will need to be locked as well. Test Case: 1. Open your all-ns.js (or all.js if using mozilla) file and add the following lines to that file lockPref("mail.server.server1.offline_download", true); lockPref("mail.server.server1.download_bodies_on_get_new_mail", true); 2. Start Mozilla Mail Client and create an account if you haven't already 3. Open Mail/News Account Prefs window Edit -> Mail/News account settings and the Offline and DiskSpace field. 4. All Offline check boxes and buttons should be greyed out and checked. Again Disk Space area does not have any prefstrings. Once prefstrings have been added they must be lockable as well.
assigning QA to myself, adding esther to Cc.
QA Contact: esther → rvelasco
adding jpm to cc.
Priority: -- → P2
Target Milestone: --- → mozilla0.9.2
this blocks bug 70623 from being verified. Need some traction on this please.
Blocks: 70623
working on it.
This is actually preference issue
Attached patch Fix to lock the preference (deleted) — Splinter Review
Need clarification regarding User Prefs "mail.server.server1.offline_download" "mail.server.server1.download_bodies_on_get_new_mail" are actually for which checkbox.
These are preferences for the offline section. User Prefs "mail.server.server1.offline_download" = When I create new folders, select them for offline "mail.server.server1.download_bodies_on_get_new_mail" = Make the messages in my Inbox available when I am working offline
Attached patch Modified fix (deleted) — Splinter Review
Why does locking a pref also cause it to be checked? Can't it be locked to an unchecked state? Rodney, correct me if I'm wrong, but I don't think you really meant this: "4. All Offline check boxes and buttons should be greyed out and checked." What about the "Select.." button that brings up the folders dialog? I suggest using a new pref. This has been done for many other buttons. Generally I've used the format: "whereever_your_section_belongs_to.disable_button.button_name" e.g "mail.disable_button.set_default_account" And then set the disabled attribute on the button if the pref is locked. Let me know if you need clarification on this. The Disk Space section is not being addressed by this patch, and I believe it should be.
For the specified testcase I described the boxes should be checked because the lockPref value is set to true for both cases... lockPref("mail.server.server1.offline_download", true); lockPref("mail.server.server1.download_bodies_on_get_new_mail", true); In a general case of course the values won't be checked if the preference is set to false, I was just trying to descibe what the user would see in the offline area if they used my testcase. As for the Select button, yes that too does need to be addressed, I described it my initial description as a select box...maybe I should have been more descriptive and called it a "Select Button".
to clarify again I meant "Select.." button not "Select Button"
Revised Test Case: Before you begin, delete your prefs.js file Linux: "~/.mozilla/your_user_name/salted directory/prefs.js" Mac: "Hard Drive: Documents : Mozilla : Profiles : Username : salted directory : prefs.js" WinNT: "C:\WinNT\Profiles\your_user_profile\Application Data\Mozilla\Profiles\Username\salted directory\prefs.js" 1. Open your all-ns.js (or all.js if using mozilla) file using any file editor and add the following lines to that file lockPref("mail.server.server1.offline_download", true); lockPref("mail.server.server1.download_bodies_on_get_new_mail", true); 2. Start Mozilla Mail Client and create an account 3. Open Mail/News Account Prefs window Edit -> Mail/News account settings and the Offline and DiskSpace field. Expected Result: All "Offline Section" check boxes should be greyed out and checked (not checked if you set the lockPref value to false). Actual Result: Boxes are checked but not locked.
Status: NEW → ASSIGNED
Whiteboard: Request R&SR
Patch looks ok to me except (as eddyk pointed out) + document.getElementById("offline.newFolder").checked = true; + document.getElementById("offline.downloadBodiesOnGetNewMail").checked = true; We shouldn't be explicitely setting these values. They are what prefs say they are and * document.getElementById("offline.downloadBodiesOnGetNewMail").checked = gImapIncomingServer.downloadBodiesOnGetNewMail; * document.getElementById("offline.newFolder").checked = gImapIncomingServer.offlineDownload; will take care of them. Rodney's test case should succeed without us having to set checked attribute explicitely to true as the pref value will be grabbed anyway. Infact having checked to true will simply force that behavior no matter what the pref value is. Let me know if there is any other purpose behind it. bhuvan
Attached patch Removed the checked lines (deleted) — Splinter Review
r=bhuvan. Please test the lock feature before you checkin. thanks. bhuvan
what about the disk-space ui elements? It doesn't look like they're being disabled.
disk-space ui elements are not related to Preferences "mail.server.server1.offline_download" "mail.server.server1.download_bodies_on_get_new_mail"
and how bout the select button? is that lockable too? If so, what is the prefstring for that button?
The initial bug report states: There are no prefstrings available for select box and Disk Space pref elements, those elements will need to be locked as well.
In the account manager, the preference is per-server based, not global, there is no prefstring associated with them. If there is any other preference should be locked based on Lock_pref, please file bugs. Please also confirm if "mail.server.server1.offline_download" is set in the lock preference, the "Select" button should be locked.
filed bug 85335 for disk space UI issue.
PDT+ per 6/12 mtg.
Whiteboard: Request R&SR → [PDT+] Request R&SR
Request SR again.
aren't you supposed to be using the pref branch to lock prefs, instead of pref service directly? see am-prefs.js, getAccountValueIsLocked() eddy k / bness might know more about this.
It would be nice to use getAccountValueIsLocked(). But getAccountValueIsLocked() requires to use "prefstr". "prefstr" is for general preference, ie global preference usage. For the account manager panel, every preference is per server based. Each preference lock has to be processed indivisually.
Every other panel using prefstr attributes is per-server or per-identity based. In AccountManager, the prefstring is not currently handled by nsPrefWindow. It is using its own code where the prefstring can be per-server based. This is done by using tokens and and allowing the prefstring handling code to substitute the server value in the prefstring. If you look at other panels, you'll see the tokens %serverkey% and %identitykey% among others. These get filled in by automatically by the token substitution code. You should be able lock the new xul elements if you know the prefstring. Soon, the custom code in AM will be replaced with an updated nsPrefWindow which should handle all this.
Yes, Seth is correct. Please do not add new code which uses nsIPref, use nsIPrefService & nsIPrefBranch instead. A rough example would look like: var prefService = Components.classes["@mozilla.org/preferences-service;1"]; prefService = prefService.getService(); prefService = prefService.QueryInterface(Components.interfaces.nsIPrefService); finalPrefString = initPrefString + "." + gIncomingServer.key + "." var pref = prefService.getBranch(finalPrefString); isDownloadLocked = pref.PrefIsLocked("offline_download"); . . . . . . isGetNewLocked = pref.PrefIsLocked("download_bodies_on_get_new_mail");
Attached patch latest fix using pref branch (deleted) — Splinter Review
You only have to get the branch once. You don't need to get it for each preference call.
Attached patch get rid of one call for branch (deleted) — Splinter Review
In your xul file, you can disable buttons using the following attributes in the button element. pref="true" preftype="bool" prefattribute="disabled" prefstring="mail.offline.disable_button.select_folder" or if you want to make it per-server based locking, add in a %serverkey% token.
You have some typos: preftrype -> preftype
Attached patch New xul prefrence (deleted) — Splinter Review
Sorry, a couple more things. 1) the prefattribute attribute for checkboxes should be "checked" not disabled. prefattribute="checked" Only buttons should be set disabled for that attribute. 2) The radiogroup (id=nntp.keepMsg) does not have a complete set of attributes. It has pref, prefattribute, and preftype but it doesn't have prefstring. It should have all 4 or none. While there may be other issues, they probably can't be addressed until I finish bug 79305 r=eddyk on the xul changes.
Looks good. r=bhuvan.
I'm totally confused as to what the complete patch is. is the complete patch just the last attachment? for my own benefit, do you test lock prefs? where do you set them as locked?
s/do you test lock prefs?/How do you test locked prefs?
The complete patch includes attachment 38648 [details] [diff] [review] and 38993. The 38648 is actually locking the preference using the xpcom, AM approach, the 38993 is for future nsIPref us to make them lockable generically. I'll make a complete patch. To test the locked preference, you still use the same lock pref in all.js. To actually test the lock by using prefstring only, we have to wait Eddy checkin bug 79305. He actually tested the xul change in his own tree.
wait, so the xul of your patch doesn't do anything until eddyk checks in? and when eddyk checks in, you'll be removing the js part of your fix? if that is the case, don't do that. just attach a patch for the current fix, and open a new bug (dependent on eddyk's fix) to remove the .js and make the .xul changes.
OK, Could we approve the js change only this time, then move the xul change to another bug.
what's the final fix? please re-attach a final fix.
Diane, Please do file a new bug on adding your XUL changes & removing the js changes contingent to eddyk's changes before checking in so that we have a track for the actual bug/fix. r=bhuvan.
a= asa@mozilla.org for checkin to the trunk. (on behalf of drivers)
Blocks: 83989
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Reopening bug. Cannot lock preferences using 2001062008 builds. Tried locking preference in all-ns.js and using a netscape.cfg file. This also caused a regression with offline prefs. Gary Chan is filing bug for this issue. Added this pref in all-ns.js file: lockPref("mail.server.server1.offline_download", true); lockPref("mail.server.server1.download_bodies_on_get_new_mail", true); Started commercial build, and preferences were checked....but not locked.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
PrefIsLocked should have been prefIsLocked. see bug #86996
a= asa@mozilla.org for checkin to frozen 0.9.2. (on behalf of drivers)
fixed. I'm able to lock these prefs now.
Status: REOPENED → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → FIXED
Attached file CFG to lockdown offline prefs (deleted) —
Using the CFG I provide above I was able to lockdown the offline prefs: "mail.server.server1.offline_download" "mail.server.server1.download_bodies_on_get_new_mail" Tested on 2001062508 Linux RH7.0 2001062303 Mac OS 9.1 (place CFG in mozilla install directory: Essential Files dir) 2001062509 Win2000 Select Button is not lockable, will file new bug to get that preference element in Mail/News prefs to lock.
Status: RESOLVED → VERIFIED
Select Button will be handled by bug 86778.
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: