Closed Bug 429015 Opened 16 years ago Closed 16 years ago

Migrate Offline & Disk Space prefpane

Categories

(SeaMonkey :: Preferences, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0a1

People

(Reporter: bugzilla, Assigned: bugzilla)

References

Details

(Whiteboard: H'n'C)

Attachments

(1 file, 4 obsolete files)

 
Whiteboard: H'n'C
Attached patch Migration of the prefpane v1 (obsolete) — Splinter Review
Assignee: nobody → aqualon
Status: NEW → ASSIGNED
Attachment #315632 - Flags: superreview?(neil)
Attachment #315632 - Flags: review?(mnyromyr)
Attached patch diff -w version of patch v1 (obsolete) — Splinter Review
Comment on attachment 315632 [details] [diff] [review]
Migration of the prefpane v1

>diff -N mailnews/base/prefs/resources/content/pref-offline.js
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ mailnews/base/prefs/resources/content/pref-offline.js	14 Apr 2008 21:19:32 -0000
It helps to diff the jar.mn change too :-P

>+        <label value="&kb.label;"
>+               id="kbLabel"/>
Why not <label id="kbLabel" value="&kb.label;"/> on one line?

>         <!-- XXX Move pref panes from below to here as they are migrated -->
>         <treeitem id="tagsItem"
>                   label="&tags.label;"
>                   prefpane="tags_pane"
>                   url="chrome://messenger/content/pref-tags.xul"
>                   helpTopic="mail-prefs-tags"/>
>+        <treeitem id="offlineItem"
>+          label="&offline.label;"
>+          prefpane="offline_pane"
>+          url="chrome://messenger/content/pref-offline.xul"
>+          helpTopic="mail_prefs_offline"/>
Indentation is wrong. Also, it says XXX Move for a reason!

sr=me with these fixed.
Attachment #315632 - Flags: superreview?(neil) → superreview+
Comment on attachment 315632 [details] [diff] [review]
Migration of the prefpane v1

>Index: mailnews/base/prefs/resources/content/pref-offline.js
>===================================================================

>+function Startup()
>+{
>+  var value = document.getElementById("mail.prompt_purge_threshhold").value;
>+  EnableTextbox("offlineCompactFolderMin", value, false);
>+}
>+
>+function EnableMailPurgeThreshhold(aValue)
>+{
>+  var focus = (document.getElementById("offlineCompactFolder") == document.commandDispatcher.focusedElement);
>+  EnableTextbox("offlineCompactFolderMin", aValue, focus);
>+}
>+
>+function EnableTextbox(aTextboxId, aValue, aFocus)
You passing "offlineCompactFolderMin" from both calls, so just drop that argument and use it explicitly in the line below.
>+{
>+  var textbox = document.getElementById(aTextboxId);
>+  var pref = document.getElementById("mail.purge_threshhold");
>+  var enable = aValue && !pref.locked;
>+
>+  textbox.disabled = !enable;
>+
>+  if (!textbox.disabled && aFocus)
You already know what !textbox.disabled is (enable) so use that instead.
>+    textbox.focus();
>+}
Attached patch Migration of the prefpane v2 (obsolete) — Splinter Review
Fixing the nits from before and using prefwindow's EnableTextBox() function.
Attachment #315632 - Attachment is obsolete: true
Attachment #319288 - Flags: review?(mnyromyr)
Attachment #315632 - Flags: review?(mnyromyr)
Attached patch diff -w version of patch v2 (obsolete) — Splinter Review
Attachment #315634 - Attachment is obsolete: true
Comment on attachment 319288 [details] [diff] [review]
Migration of the prefpane v2

>Index: mailnews/base/prefs/resources/content/pref-offline.xul
>===================================================================
>+        <label value="&textStartUp;" control="whenStartingUp"/>

We need an accesskey here.
(Unfortunately I didn't spot that in the patch removing the former accesskeys. :-/)

r=me with that.
Attachment #319288 - Flags: review?(mnyromyr) → review+
Patch includes accesskey as mentioned in previous review comment.
Attachment #319288 - Attachment is obsolete: true
Attachment #319289 - Attachment is obsolete: true
Comment on attachment 319454 [details] [diff] [review]
Migration of the prefpane v3

Landed on trunk with r=me and sr=Neil.
Attachment #319454 - Flags: superreview+
Attachment #319454 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.