Bug 343832 - New history prefs UI needs a little more work. r=mconnor

browser/components/preferences/privacy.js

@@ -47,17 +47,7 @@ var gPrivacyPane = {
    */
   init: function ()
   {
-    this.historyDaysPrefChanged();
-
-    var self = this;
-    function checkboxChanged() {
-      self.onchangeHistoryDaysCheckbox();
-    }
-    var historyDaysCheckbox = document.getElementById("rememberHistoryDays");
-    historyDaysCheckbox.addEventListener("CheckboxStateChange",
-                                         checkboxChanged,
-                                         false);
-
+    this._updateHistoryDaysUI();
     this.updateClearNowButtonLabel();
   },
 
@@ -68,6 +58,9 @@ var gPrivacyPane = {
    *
    * browser.history_expire_days
    * - the number of days of history to remember
+   * browser.history_expire_days.mirror
+   * - a preference whose value mirrors that of browser.history_expire_days, to
+   *   make the "days of history" checkbox easier to code
    * browser.formfill.enable
    * - true if entries in forms and the search bar should be saved, false
    *   otherwise
@@ -80,55 +73,59 @@ var gPrivacyPane = {
    *     2 means never remove downloads
    */
 
-  // XXXjwalden the UI for days of history to remember is totally broken -- I blame beltzner
-
   /**
-   * Enables/disables the history days textbox based on the state of the
-   * associated checkbox.
+   * Initializes the days-of-history mirror preference and connects it to the
+   * days-of-history checkbox so that updates to the textbox are transmitted to
+   * the real days-of-history preference.
    */
-  historyDaysPrefChanged: function ()
+  _updateHistoryDaysUI: function ()
   {
     var pref = document.getElementById("browser.history_expire_days");
+    var mirror = document.getElementById("browser.history_expire_days.mirror");
     var textbox = document.getElementById("historyDays");
     var checkbox = document.getElementById("rememberHistoryDays");
 
-    var prefVal = pref.value;
-    textbox.disabled = (prefVal == 0);
-    textbox.value = prefVal;
-    checkbox.checked = (prefVal != 0);
-  },
-
-  /**
-   * Handles enabling/disabling the "days of history" textbox based on the state
-   * of the associated checkbox.
-   */
-  onchangeHistoryDaysCheckbox: function (event)
-  {
-    var textbox = document.getElementById("historyDays");
-    var checkbox = document.getElementById("rememberHistoryDays");
+    // handle mirror non-existence or mirror/pref unsync
+    if (mirror.value === null || mirror.value != pref.value)
+      mirror.value = pref.value ? pref.value : pref.defaultValue;
 
+    checkbox.checked = (pref.value > 0);
     textbox.disabled = !checkbox.checked;
-    if (!checkbox.checked) {
-      var pref = document.getElementById("browser.history_expire_days");
-      pref.value = 0;
-    }
+
+    // hook up textbox to mirror preference and force a preference read
+    textbox.setAttribute("onsynctopreference", "return gPrivacyPane._writeHistoryDaysMirror();");
+    textbox.setAttribute("preference", "browser.history_expire_days.mirror");
+    mirror.updateElements();
   },
 
   /**
-   * Sets the value of browser.history_expire_days appropriately based on the
-   * value displayed in UI.
+   * Stores the days of history to the actual days-of-history preference and
+   * returns that value, to be stored in the mirror preference.
    */
-  changeHistoryDays: function ()
+  _writeHistoryDaysMirror: function ()
   {
     var pref = document.getElementById("browser.history_expire_days");
-    var historyDays = document.getElementById("historyDays");
+    var textbox = document.getElementById("historyDays");
+    pref.value = textbox.value;
 
-    // allow deletion of everything before typing a new value
-    if (historyDays.value == "")
-      return;
+    // don't override the value in the textbox
+    return undefined;
+  },
 
-    var uiValue = parseInt(historyDays.value, 10);
-    pref.value = isNaN(uiValue) ? 0 : uiValue;
+  /**
+   * Responds to the checking or unchecking of the days-of-history UI, storing
+   * the appropriate value to the days-of-history preference and enabling or
+   * disabling the number textbox as appropriate.
+   */
+  onchangeHistoryDaysCheck: function ()
+  {
+    var pref = document.getElementById("browser.history_expire_days");
+    var mirror = document.getElementById("browser.history_expire_days.mirror");
+    var textbox = document.getElementById("historyDays");
+    var checkbox = document.getElementById("rememberHistoryDays");
+
+    pref.value = checkbox.checked ? mirror.value : 0;
+    textbox.disabled = !checkbox.checked;
   },
 
   /**

browser/components/preferences/privacy.xul

@@ -65,8 +65,10 @@
       <!-- History -->
       <preference id="browser.history_expire_days"
                   name="browser.history_expire_days"
-                  type="int"
-                  onchange="gPrivacyPane.historyDaysPrefChanged();"/>
+                  type="int"/>
+      <preference id="browser.history_expire_days.mirror"
+                  name="browser.history_expire_days.mirror"
+                  type="int"/>
       <preference id="browser.formfill.enable"
                   name="browser.formfill.enable"
                   type="bool"/>
@@ -96,9 +98,8 @@
         <checkbox id="rememberHistoryDays"
                   label="&rememberBefore.label;"
                   accesskey="&rememberBefore.accesskey;"
-                  onchange="gPrivacyPane.onchangeHistoryDaysCheckbox();"/>
-        <textbox id="historyDays" size="2"
-                 onkeyup="gPrivacyPane.changeHistoryDays();"/>
+                  oncommand="gPrivacyPane.onchangeHistoryDaysCheck();"/>
+        <textbox id="historyDays" size="3"/>
         <label>&rememberAfter.label;</label>
       </hbox>
       <checkbox id="rememberForms"
@@ -128,8 +129,8 @@
                 preference="pref.privacy.disable_button.cookie_exceptions"/>
       </hbox>
       <hbox id="keepRow">
-        <hbox id="keepBox" align="center">
-          <label id="keepUntil" class="indent"
+        <hbox id="keepBox" align="center" class="indent">
+          <label id="keepUntil"
                  control="keepCookiesUntil"
                  accesskey="&keepUntil.accesskey;">&keepUntil.label;</label>
           <menulist id="keepCookiesUntil"