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)
SeaMonkey
MailNews: Message Display
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.2
People
(Reporter: rvelasco, Assigned: dianesun)
References
()
Details
(Whiteboard: [PDT+] Request R&SR)
Attachments
(11 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
application/octet-stream
|
Details |
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.
Reporter | ||
Comment 1•24 years ago
|
||
assigning QA to myself, adding esther to Cc.
QA Contact: esther → rvelasco
Reporter | ||
Comment 2•24 years ago
|
||
adding jpm to cc.
Updated•24 years ago
|
Priority: -- → P2
Target Milestone: --- → mozilla0.9.2
Reporter | ||
Comment 3•24 years ago
|
||
this blocks bug 70623 from being verified. Need some traction on this please.
Blocks: 70623
Need clarification regarding
User Prefs
"mail.server.server1.offline_download"
"mail.server.server1.download_bodies_on_get_new_mail"
are actually for which checkbox.
Reporter | ||
Comment 8•24 years ago
|
||
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
Comment 10•24 years ago
|
||
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.
Reporter | ||
Comment 11•24 years ago
|
||
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".
Reporter | ||
Comment 12•24 years ago
|
||
to clarify again I meant "Select.." button not "Select Button"
Reporter | ||
Comment 13•24 years ago
|
||
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.
Comment 14•24 years ago
|
||
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
Assignee | ||
Comment 15•24 years ago
|
||
Comment 16•24 years ago
|
||
r=bhuvan.
Please test the lock feature before you checkin. thanks.
bhuvan
Comment 17•24 years ago
|
||
what about the disk-space ui elements? It doesn't look like they're being
disabled.
Assignee | ||
Comment 18•24 years ago
|
||
disk-space ui elements are not related to Preferences
"mail.server.server1.offline_download"
"mail.server.server1.download_bodies_on_get_new_mail"
Reporter | ||
Comment 19•24 years ago
|
||
and how bout the select button? is that lockable too? If so, what is the
prefstring for that button?
Comment 20•24 years ago
|
||
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.
Assignee | ||
Comment 21•24 years ago
|
||
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.
Reporter | ||
Comment 22•24 years ago
|
||
filed bug 85335 for disk space UI issue.
Assignee | ||
Comment 24•24 years ago
|
||
Request SR again.
Comment 25•24 years ago
|
||
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.
Assignee | ||
Comment 26•24 years ago
|
||
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.
Comment 27•24 years ago
|
||
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.
Comment 28•24 years ago
|
||
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");
Assignee | ||
Comment 29•24 years ago
|
||
Comment 30•24 years ago
|
||
You only have to get the branch once. You don't need to get it for each
preference call.
Assignee | ||
Comment 31•24 years ago
|
||
Comment 32•24 years ago
|
||
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.
Assignee | ||
Comment 33•24 years ago
|
||
Comment 34•24 years ago
|
||
You have some typos:
preftrype -> preftype
Assignee | ||
Comment 35•24 years ago
|
||
Comment 36•24 years ago
|
||
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.
Assignee | ||
Comment 37•24 years ago
|
||
Comment 38•24 years ago
|
||
Looks good. r=bhuvan.
Comment 39•24 years ago
|
||
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?
Comment 40•24 years ago
|
||
s/do you test lock prefs?/How do you test locked prefs?
Assignee | ||
Comment 41•24 years ago
|
||
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.
Assignee | ||
Comment 42•24 years ago
|
||
Comment 43•24 years ago
|
||
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.
Assignee | ||
Comment 44•24 years ago
|
||
OK, Could we approve the js change only this time, then move the xul change
to another bug.
Comment 45•24 years ago
|
||
what's the final fix? please re-attach a final fix.
Assignee | ||
Comment 46•24 years ago
|
||
Comment 47•24 years ago
|
||
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.
Comment 48•24 years ago
|
||
sr=sspitzer
Comment 49•24 years ago
|
||
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
Blocks: 83989
Assignee | ||
Comment 50•24 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 51•24 years ago
|
||
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 → ---
Comment 52•24 years ago
|
||
PrefIsLocked should have been prefIsLocked.
see bug #86996
Comment 53•24 years ago
|
||
a= asa@mozilla.org for checkin to frozen 0.9.2.
(on behalf of drivers)
Comment 54•24 years ago
|
||
fixed. I'm able to lock these prefs now.
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 55•24 years ago
|
||
Reporter | ||
Comment 56•24 years ago
|
||
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
Assignee | ||
Comment 57•24 years ago
|
||
Select Button will be handled by bug 86778.
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•