429015 - Migrate Offline & Disk Space prefpane

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Bruno 'Aqualon' Escherl]

Comment 1

[Bruno 'Aqualon' Escherl]

Attachment: Migration of the prefpane v1

Comment 2

[Bruno 'Aqualon' Escherl]

Attachment: diff -w version of patch v1

Comment 3

[[e-mail]]

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.

Comment 4

[Ian Neal]

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();
>+}

Comment 5

[Bruno 'Aqualon' Escherl]

Attachment: Migration of the prefpane v2

Fixing the nits from before and using prefwindow's EnableTextBox() function.

Comment 6

[Bruno 'Aqualon' Escherl]

Attachment: diff -w version of patch v2

Comment 7

[Karsten Düsterloh]

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.

Comment 8

[Bruno 'Aqualon' Escherl]

Attachment: Migration of the prefpane v3

Patch includes accesskey as mentioned in previous review comment.

Comment 9

[Karsten Düsterloh]

Comment on attachment 319454 [details] [diff] [review]
Migration of the prefpane v3

Landed on trunk with r=me and sr=Neil.