Closed Bug 85335 Opened 24 years ago Closed 23 years ago

Disk Space UI Elements need to be lockable

Categories

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

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.4

People

(Reporter: rvelasco, Assigned: eddyk)

References

(Blocks 1 open bug, )

Details

Attachments

(4 files)

Per dianesuns request on bug 82805, filing bug for disk space UI elements need to be lockable. The prefstrings for these elements in 4.x were "mail.max_size" = Do not download any message larger than: XX K (XX denotes number) "mail.purge_threshhold" = Compact folders when it will save over: XX K
QA over to me
QA Contact: esther → rvelasco
Blocks: 70623
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.9.3
adding gary chan to Cc.
Is "mail.max_size" a global preference or a per-server one?
by looking at the UI "mail.max_size" appears to be a per server type of pref.
In that case, in 6.x, should that be "mail.server.severxx.max_size"?
In 4.x there is Disk Space preference in Mailnews&Newsgroups. Some preferences for newsgroup are not secified or request here. Please list all your reqest preferences. Thanks.
Keywords: nsenterprise
Actually, mail.purge_threshhold is for the text field, and mail.prompt_purge_threshold is for the checkbox. In my previous bug fix, I added the prefstring the these preferences. But the text field needs to be locked when the the checkbox is unchecked, the same requirement as 87177. the mail.max_size is also the text field which is already locked by the checkbox in 87177 but we need to add further prefstring for the checkbox/radio preferences in Disk Space.
Reassigning.
Assignee: dianesun → eddyk
Status: ASSIGNED → NEW
Depends on: 86014
I'm working on a patch that should close this and bug 86778. The prefstrings that are used to lock the elements are: mail.server.[serverkey].limit_message_size mail.server.[serverkey].max_size mail.server.[serverkey].downloadUnreadOnly mail.server.[serverkey].downloadByDate mail.server.[serverkey].ageLimit mail.server.[serverkey].retainBy mail.server.[serverkey].daysToKeepHdrs mail.server.[serverkey].numHdrsToKeep mail.server.[serverkey].keepUnreadOnly mail.server.[serverkey].daysToKeepBodies mail.server.[serverkey].disable_button.selectFolder
Attached patch patch to lock panel elements (deleted) — Splinter Review
Description of patch: .js changes - do a lock/unlock check on page init. Pretty straightforward .xul changes - This is from bug 86014. It removes some code and changes some existing stuff. Apparently someone added (or moved) code into the panel without checking the existing ids. There was a conflict with 2 identical ids causing the element to stop working properly.
just talked to srilatha and got an r=
can you get an r= from the module owner (Bhuvan) first? Once he's looked at it, i'll super review it.
Eddy, patch looks OK. I think we can go for couple of minor improvements here. 1. "var pref = prefService.getBranch(finalPrefString);" , we should proabbly make pref a global (gPref or a more context sensitive name). That way we can skip passing that var all the time to disableIfLocked(). 2. disableIfLocked() is called several times. Instead, we can just call it once and pass it an array of prefstring and id pairs. Then , we can simply loop in disableIfLocked. We should add comments on the top that array indicating any new items to be added to the list of locakable pref should be added to that list. 3. I noticed some changes in xul file that are strictly not related to locking prefs (correct me if I am wrong). If you need to get them in, please post screenshots for POP, IMAP and News panels as a part our regular UI review change approvals. thanks bhuvan
- Did the suggested changes. Thanks for the excellent feedback. - You are correct, the xul file is not strictly for locking. I found that one of the checkboxes was broken and that diane's change was still not checked in. I thought I would incorporate her change and get it checked in as well. The existing bug and screenshots are in bug 86014. The only changes I made were to update from the old xul syntax for box to the new hbox. Would you prefer that I leave that part out and let the original bug stand? (She is aware of my change and intent to check it in)
Eddy, Did you forget to post the updated patch here..? Please post the new one so that you can go the next step i.e., the super review. With respect to the XUL file changes, I think checkins should happen from corresponding bugs with r & sr= done in both bugs. I noticed that the other bug 86014 has already been reviewed. Just get sr there and you(/diane) should be good to go. Sorry for the extra work. But, I think it will be a better way to go about it (also I noticed that super reviewers are different bug these 2 bugs, so it is better to stick to the domain the bug to make reviews go faster).
May you want to get document.getElementById(prefstrArray[i].id) in this into a local var like offlineElement or something (in routine disableIfLocked) and then use offlineElement.disabled() & offlineElement.removeAttribute(). r=bhuvan
everything looks good. sr-mscott
set milestone to 0.9.4
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Blocks: 70538
checked-in fix.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
I have commented onLockPreference() out in am-offline.js since it causes regression. Please rework on is after 86014 is checked in. By the way, could you move the global var gPref to the top?
re-opening
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Description of changes: * moved gPref to top of file. * added global associative array to keep track of prefs -> locked state. * added checks in existing logic that disables/enable elements to also check the global associative array to avoid removing the disabled attribute when appropriate. * added some code comments Since Bhuvan is on vacation still, can I get someone else to review this patch? Diane?
r=dianesun on 45237
check-in
Status: REOPENED → RESOLVED
Closed: 24 years ago23 years ago
Resolution: --- → FIXED
All prefs lockable except for the checkbox "mail.server.[serverkey].daysToKeepBodies" This is part of the mail/news Offline and Disk Space section. Eddy, I believe this is locked in the backend because the value does not change if I uncheck the box or change the value. I also had another question as to how to enable a radio box for the pref.... "mail.server.[serverkey].retainBy" Through my testing I found that this preference controls the radio box disabling for all elements listed in the radio box selection. The default value for this pref is an int = 1. By default this chooses the "Keep all messages" radio box option in the disk space section. What if I wanted the "keep messages which have arrived with the past XX days" radio box to be pressed and locked down. I could lockdown the XX days by specifying this pref "mail.server.server4.daysToKeepHdrs", but this doesn't set the radio box value.
The UI bug for the pref 'remove message bodies older than x days' is bug 91560.
Cancel my last comments...figured out that the radio buttons for the news offline prefs are working, but in an unorthodox way. lockPref("mail.server.server3.retainBy", 2); /* This pref refers to the radio button..."Keep messages which have arrived with the past", this does not however control the value of the textbox */ lockPref("mail.server.server3.retainBy", 1); /* This pref refers to the radio button..."keep all messages"...which is the actually the middle radiobox element */ lockPref("mail.server.server3.retainBy", 3); /* This pref refers to the radio button "Keep the Newest XX messages"...however this does not refer to the numrical value for XX...thats controlled by another pref */ Marking this verified. Verified on branch bits: Linux: 20010927 Win2K: 20010927 MacOS 9.1: 20010926MacOS X: 20010927
Status: RESOLVED → VERIFIED
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: