Move handling of dialog preferences to Preferences class

chrome/browser/resources/options2/pref_ui.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// TODO(jhawkins): Add dialog-pref support to all preference controls.
-
 cr.define('options', function() {
 
   var Preferences = options.Preferences;
 
   /**
    * Allows an element to be disabled for several reasons.
    * The element is disabled if at least one reason is true, and the reasons
    * can be set separately.
    * @private
    * @param {!HTMLElement} el The element to update.
@@ skipping 24 matching lines @@
     el.disabled = Object.keys(el.disabledReasons).length > 0;
   }
 
   /**
    * Helper function to update element's state from pref change event.
    * @private
    * @param {!HTMLElement} el The element to update.
    * @param {!Event} event The pref change event.
    */
   function updateElementState_(el, event) {
-    el.controlledBy = null;
-
-    if (!event.value)
-      return;
-
     updateDisabledState_(el, 'notUserModifiable', event.value.disabled);
-
     el.controlledBy = event.value.controlledBy;
-
     OptionsPage.updateManagedBannerVisibility();
   }
 
   /////////////////////////////////////////////////////////////////////////////
   // PrefCheckbox class:
   // TODO(jhawkins): Refactor all this copy-pasted code!
 
   // Define a constructor that uses an input element as its underlying element.
   var PrefCheckbox = cr.ui.define('input');
 
   PrefCheckbox.prototype = {
     // Set up the prototype chain
     __proto__: HTMLInputElement.prototype,
 
     /**
-     * The stored value of the preference that this checkbox controls.
-     * @type {boolean}
-     */
-    prefValue_: null,
-
-    /**
      * Initialization function for the cr.ui framework.
      */
     decorate: function() {
       this.type = 'checkbox';
       var self = this;
 
       self.initializeValueType(self.getAttribute('value-type'));
 
       // Listen to pref changes.
       Preferences.getInstance().addEventListener(this.pref, function(event) {
-        self.prefValue_ = Boolean(event.value.value);
-        self.resetPrefState();
-
+        var value = event.value.value;
+        self.checked = self.inverted_pref ? !value : value;
         updateElementState_(self, event);
       });
 
       // Listen to user events.
-      this.addEventListener('change', function(e) {
-        if (self.customChangeHandler(e))
+      this.addEventListener('change', function(event) {
+        if (this.customChangeHandler(event))

[James Hawkins, 2012/08/02 20:33:32]

We have an inconsistency here: we use |self| above and |this| here.

[bartfab (slow), 2012/08/31 14:14:30]

Done.

           return;
 
-        if (!this.dialogPref)
-          this.updatePreferenceValue_();
+        var value = this.inverted_pref ? !this.checked : this.checked;
+        switch (this.valueType) {
+          case 'number':
+            Preferences.setIntegerPref(this.pref, Number(value),
+                                       !this.dialogPref, this.metric);
+            break;
+          case 'boolean':
+            Preferences.setBooleanPref(this.pref, value,
+                                       !this.dialogPref, this.metric);
+            break;
+        }
       });
     },
 
     /**
-     * Update the preference value based on the checkbox state.
-     * @private
-     */
-    updatePreferenceValue_: function() {
-      var value = this.inverted_pref ? !this.checked : this.checked;
-      switch (this.valueType) {
-        case 'number':
-          Preferences.setIntegerPref(this.pref, Number(value), this.metric);
-          break;
-        case 'boolean':
-          Preferences.setBooleanPref(this.pref, value, this.metric);
-          break;
-      }
-    },
-
-    /**
-     * Called by SettingsDialog to save the preference.
-     */
-    savePrefState: function() {
-      this.updatePreferenceValue_();
-    },
-
-    /**
-     * Called by SettingsDialog to reset the UI to match the current preference
-     * value.
-     */
-    resetPrefState: function() {
-      this.checked = this.inverted_pref ? !this.prefValue_ : this.prefValue_;
-    },
-
-    /**
      * Sets up options in checkbox element.
      * @param {String} valueType The preference type for this checkbox.
      */
     initializeValueType: function(valueType) {
       this.valueType = valueType || 'boolean';
     },
 
     /**
      * See |updateDisabledState_| above.
      */
@@ skipping 13 matching lines @@
     },
   };
 
   /**
    * The preference name.
    * @type {string}
    */
   cr.defineProperty(PrefCheckbox, 'pref', cr.PropertyKind.ATTR);
 
   /**
    * A special preference type specific to dialogs. These preferences are reset

[James Hawkins, 2012/08/02 20:33:32]

This sentence now doesn't make as much sense.  I'd phrase it as 1) the prefs are
saved only if the user confirms the dialog and 2) if the user does not confirm
the dialog, the UI elements are reset.

[bartfab (slow), 2012/08/31 14:14:30]

Done.

-   * when the dialog is shown and are not saved until the user confirms the
+   * when the dialog is closed and are not saved until the user confirms the
    * dialog.
    * @type {boolean}
    */
   cr.defineProperty(PrefCheckbox, 'dialogPref', cr.PropertyKind.BOOL_ATTR);
 
   /**
    * Whether the preference is controlled by something else than the user's
    * settings (either 'policy' or 'extension').
    * @type {string}
    */
@@ skipping 31 matching lines @@
     decorate: function() {
       this.type = 'radio';
       var self = this;
 
       // Listen to preference changes.
       Preferences.getInstance().addEventListener(this.pref,
           function(event) {
             if (self.customChangeHandler(event))
               return;
             self.checked = String(event.value.value) == self.value;
-            self.storedValue_ = self.checked;
 
             updateElementState_(self, event);
           });
 
-      // Dialog preferences are not saved until savePrefState() is explicitly
-      // called.
-      if (!this.dialogPref)
-        this.onchange = this.savePrefState.bind(this);
+      // Listen to user events.
+      this.addEventListener('change', function(event) {
+        if (this.customChangeHandler(event))
+          return;
+
+        if (this.value == 'true' || this.value == 'false') {
+          var value = String(this.value);
+          Preferences.setBooleanPref(this.pref, value == String(this.checked),
+                                     !this.dialogPref, this.metric);
+        } else {
+          Preferences.setIntegerPref(this.pref, parseInt(this.value, 10),
+                                     !this.dialogPref, this.metric);
+        }
+      });
     },
 
     /**
-     * Resets the input to the stored value.
-     */
-    resetPrefState: function() {
-      this.checked = this.storedValue_;
-    },
-
-    /**
-     * Saves the value of the input back into the preference. May be called
-     * directly to save dialog preferences.
-     */
-    savePrefState: function() {
-      this.storedValue_ = this.checked;
-      if (this.value == 'true' || this.value == 'false') {
-        var value = String(this.value);
-        Preferences.setBooleanPref(this.pref, value == String(this.checked),
-                                   this.metric);
-      } else {
-        Preferences.setIntegerPref(this.pref, parseInt(this.value, 10),
-                                   this.metric);
-      }
-    },
-
-    /**
      * See |updateDisabledState_| above.
      */
     setDisabled: function(reason, disabled) {
       updateDisabledState_(this, reason, disabled);
     },
 
     /**
      * This method is called first while processing an onchange event. If it
      * returns false, regular onchange processing continues (setting the
      * associated pref, etc). If it returns true, the rest of the onchange is
      * not performed. I.e., this works like stopPropagation or cancelBubble.
      * @param {Event} event Change event.
      */
     customChangeHandler: function(event) {
       return false;
     },
   };
 
   /**
    * The preference name.
    * @type {string}
    */
   cr.defineProperty(PrefRadio, 'pref', cr.PropertyKind.ATTR);
 
   /**
    * A special preference type specific to dialogs. These preferences are reset
-   * when the dialog is shown and are not saved until the user confirms the
+   * when the dialog is closed and are not saved until the user confirms the
    * dialog.
    * @type {boolean}
    */
   cr.defineProperty(PrefRadio, 'dialogPref', cr.PropertyKind.BOOL_ATTR);
 
   /**
    * Whether the preference is controlled by something else than the user's
    * settings (either 'policy' or 'extension').
    * @type {string}
    */
@@ skipping 25 matching lines @@
           function(event) {
             self.value = event.value.value;
 
             updateElementState_(self, event);
           });
 
       // Listen to user events.
       this.addEventListener('change',
           function(e) {
             if (this.validity.valid) {
-              Preferences.setIntegerPref(self.pref, self.value, self.metric);
+              Preferences.setIntegerPref(self.pref, self.value,
+                                         !self.dialogPref, self.metric);
             }
           });
     },
 
     /**
      * See |updateDisabledState_| above.
      */
     setDisabled: function(reason, disabled) {
       updateDisabledState_(this, reason, disabled);
     },
   };
 
   /**
    * The preference name.
    * @type {string}
    */
   cr.defineProperty(PrefNumeric, 'pref', cr.PropertyKind.ATTR);
 
   /**
+   * A special preference type specific to dialogs. These preferences are reset
+   * when the dialog is closed and are not saved until the user confirms the
+   * dialog.
+   * @type {boolean}
+   */
+  cr.defineProperty(PrefRadio, 'dialogPref', cr.PropertyKind.BOOL_ATTR);
+
+  /**
    * Whether the preference is controlled by something else than the user's
    * settings (either 'policy' or 'extension').
    * @type {string}
    */
   cr.defineProperty(PrefNumeric, 'controlledBy', cr.PropertyKind.ATTR);
 
   /**
    * The user metric string.
    * @type {string}
    */
@@ skipping 13 matching lines @@
      * Initialization function for the cr.ui framework.
      */
     decorate: function() {
       this.type = 'number';
       PrefNumeric.prototype.decorate.call(this);
 
       // Listen to user events.
       this.addEventListener('input',
           function(e) {
             if (this.validity.valid) {
-              Preferences.setIntegerPref(self.pref, self.value, self.metric);
+              Preferences.setIntegerPref(self.pref, self.value,
+                                         !self.dialogPref, self.metric);
             }
           });
     },
 
     /**
      * See |updateDisabledState_| above.
      */
     setDisabled: function(reason, disabled) {
       updateDisabledState_(this, reason, disabled);
     },
@@ skipping 39 matching lines @@
       this.onkeyup = this.onmouseup = this.onInputUp_.bind(this);
     },
 
     /**
      * Event listener that updates the UI when the underlying pref changes.
      * @param {Event} event The event that details the pref change.
      * @private
      */
     onPrefChange_: function(event) {
       var value = event.value.value;
-      if (value != undefined)
-        this.value = this.valueMap ? this.valueMap.indexOf(value) : value;
+      this.value = this.valueMap ? this.valueMap.indexOf(value) : value;
     },
 
     /**
      * onchange handler that sets the pref when the user changes the value of
      * the input element.
      * @private
      */
     onChange_: function(event) {
       if (this.continuous)
         this.setRangePref_();
 
       if (this.notifyChange)
         this.notifyChange(this, this.mapValueToRange_(this.value));
     },
 
     /**
      * Sets the integer value of |pref| to the value of this element.
      * @private
      */
     setRangePref_: function() {
-      Preferences.setIntegerPref(
-          this.pref, this.mapValueToRange_(this.value), this.metric);
-
-      if (this.notifyPrefChange)
-        this.notifyPrefChange(this, this.mapValueToRange_(this.value));
+      Preferences.setIntegerPref(this.pref, this.mapValueToRange_(this.value),
+                                 !this.dialogPref, this.metric);
     },
 
     /**
      * onkeyup/onmouseup handler that modifies the pref if |continuous| is
      * false.
      * @private
      */
     onInputUp_: function(event) {
       if (!this.continuous)
         this.setRangePref_();
@@ skipping 26 matching lines @@
     },
   };
 
   /**
    * The preference name.
    * @type {string}
    */
   cr.defineProperty(PrefRange, 'pref', cr.PropertyKind.ATTR);
 
   /**
+   * A special preference type specific to dialogs. These preferences are reset
+   * when the dialog is closed and are not saved until the user confirms the
+   * dialog.
+   * @type {boolean}
+   */
+  cr.defineProperty(PrefRadio, 'dialogPref', cr.PropertyKind.BOOL_ATTR);
+
+  /**
    * Whether the preference is controlled by something else than the user's
    * settings (either 'policy' or 'extension').
    * @type {string}
    */
   cr.defineProperty(PrefRange, 'controlledBy', cr.PropertyKind.ATTR);
 
   /**
    * The user metric string.
    * @type {string}
    */
   cr.defineProperty(PrefRange, 'metric', cr.PropertyKind.ATTR);
 
   /////////////////////////////////////////////////////////////////////////////
   // PrefSelect class:
 
   // Define a constructor that uses a select element as its underlying element.
   var PrefSelect = cr.ui.define('select');
 
   PrefSelect.prototype = {
     // Set up the prototype chain
     __proto__: HTMLSelectElement.prototype,
 
     /**
-     * @type {string} The stored value of the preference that this select
-     *     controls.
-     */
-    prefValue_: null,
-
-    /**
     * Initialization function for the cr.ui framework.
     */
     decorate: function() {
       var self = this;
 
       // Listen to pref changes.
       Preferences.getInstance().addEventListener(this.pref, function(event) {
         // Make sure |value| is a string, because the value is stored as a
         // string in the HTMLOptionElement.
         value = event.value.value.toString();
 
         updateElementState_(self, event);
-        self.prefValue_ = value;
-        self.resetPrefState();
+
+        var found = false;
+        for (var i = 0; i < self.options.length; i++) {
+          if (self.options[i].value == self.prefValue_) {
+            self.selectedIndex = i;
+            found = true;
+          }
+        }
+
+        // Item not found, select first item.
+        if (!found)
+          self.selectedIndex = 0;
+
+        if (this.onchange)
+          self.onchange(event);
       });
 
       // Listen to user events.
       this.addEventListener('change', function(event) {
-        if (!self.dialogPref)
-          self.updatePreference_(self.prefValue_);
+        if (!this.dataType) {
+          console.error('undefined data type for <select> pref');
+          return;
+        }
+
+        var prefValue = this.options[this.selectedIndex].value;
+        switch (this.dataType) {
+          case 'number':
+            Preferences.setIntegerPref(this.pref, prefValue,
+                                       !this.dialogPref, this.metric);
+            break;
+          case 'double':
+            Preferences.setDoublePref(this.pref, prefValue,
+                                      !this.dialogPref, this.metric);
+            break;
+          case 'boolean':
+            var value = (prefValue == 'true');
+            Preferences.setBooleanPref(this.pref, value,
+                                       !this.dialogPref, this.metric);
+            break;
+          case 'string':
+            Preferences.setStringPref(this.pref, prefValue,
+                                      !this.dialogPref, this.metric);
+            break;
+          default:
+            console.error('unknown data type for <select> pref: ' +
+                          this.dataType);
+        }
       });
     },
 
     /**
-     * Resets the input to the stored value.
-     */
-    resetPrefState: function() {
-      var found = false;
-      for (var i = 0; i < this.options.length; i++) {
-        if (this.options[i].value == this.prefValue_) {
-          this.selectedIndex = i;
-          found = true;
-        }
-      }
-
-      // Item not found, select first item.
-      if (!found)
-        this.selectedIndex = 0;
-
-      if (this.onchange)
-        this.onchange(event);
-    },
-
-    /**
-     * Updates the preference to the currently selected value.
-     */
-    updatePreference_: function() {
-      if (!this.dataType) {
-        console.error('undefined data type for <select> pref');
-        return;
-      }
-
-      var prefValue = this.options[this.selectedIndex].value;
-      switch (this.dataType) {
-        case 'number':
-          Preferences.setIntegerPref(this.pref, prefValue, this.metric);
-          break;
-        case 'double':
-          Preferences.setDoublePref(this.pref, prefValue, this.metric);
-          break;
-        case 'boolean':
-          var value = (prefValue == 'true');
-          Preferences.setBooleanPref(this.pref, value, this.metric);
-          break;
-        case 'string':
-          Preferences.setStringPref(this.pref, prefValue, this.metric);
-          break;
-        default:
-          console.error('unknown data type for <select> pref: ' +
-                        this.dataType);
-      }
-    },
-
-    /**
-     * Called by SettingsDialog to save the stored value to preferences.
-     */
-    savePrefState: function() {
-      this.updatePreference_();
-    },
-
-    /**
      * See |updateDisabledState_| above.
      */
     setDisabled: function(reason, disabled) {
       updateDisabledState_(this, reason, disabled);
     },
   };
 
   /**
    * The preference name.
    * @type {string}
    */
   cr.defineProperty(PrefSelect, 'pref', cr.PropertyKind.ATTR);
 
   /**
    * Whether the preference is controlled by something else than the user's
    * settings (either 'policy' or 'extension').
    * @type {string}
    */
   cr.defineProperty(PrefSelect, 'controlledBy', cr.PropertyKind.ATTR);
 
   /**
    * A special preference type specific to dialogs. These preferences are reset
-   * when the dialog is shown and are not saved until the user confirms the
+   * when the dialog is closed and are not saved until the user confirms the
    * dialog.
    * @type {boolean}
    */
   cr.defineProperty(PrefSelect, 'dialogPref', cr.PropertyKind.BOOL_ATTR);
 
   /**
    * The user metric string.
    * @type {string}
    */
   cr.defineProperty(PrefSelect, 'metric', cr.PropertyKind.ATTR);
 
   /**
    * The data type for the preference options.
    * @type {string}
    */
   cr.defineProperty(PrefSelect, 'dataType', cr.PropertyKind.ATTR);
 
   /////////////////////////////////////////////////////////////////////////////
   // PrefTextField class:
 
   // Define a constructor that uses an input element as its underlying element.
   var PrefTextField = cr.ui.define('input');
 
   PrefTextField.prototype = {
     // Set up the prototype chain
     __proto__: HTMLInputElement.prototype,
 
     /**
-     * @type {string} The stored value of the preference that this text field
-     * controls.
-     */
-    prefValue_: null,
-
-    /**
-     * Saves the value of the input back into the preference. May be called
-     * directly to save dialog preferences.
-     */
-    savePrefState: function() {
-      switch (this.dataType) {
-        case 'number':
-          Preferences.setIntegerPref(this.pref, this.value, this.metric);
-          break;
-        case 'double':
-          Preferences.setDoublePref(this.pref, this.value, this.metric);
-          break;
-        case 'url':
-          Preferences.setURLPref(this.pref, this.value, this.metric);
-          break;
-        default:
-          Preferences.setStringPref(this.pref, this.value, this.metric);
-          break;
-      }
-    },
-
-    /**
-     * Resets the input to the stored value.
-     */
-    resetPrefState: function() {
-      this.value = this.prefValue_;
-    },
-
-    /**
      * Initialization function for the cr.ui framework.
      */
     decorate: function() {
       var self = this;
 
       // Listen to pref changes.
       Preferences.getInstance().addEventListener(this.pref,
           function(event) {
             self.value = event.value.value;
 
             updateElementState_(self, event);
-
-            self.prefValue_ = self.value;
           });
 
       // Listen to user events.
-      if (!this.dialogPref)
-        this.addEventListener('change', this.savePrefState.bind(this));
+      this.addEventListener('change', function(event) {
+        switch (this.dataType) {

[James Hawkins, 2012/08/02 20:33:32]

I'm not a fan of inlining large functions, so please keep these pulled out and
use bind.

[bartfab (slow), 2012/08/31 14:14:30]

Done.

+          case 'number':
+            Preferences.setIntegerPref(this.pref, this.value,
+                                       !this.dialogPref, this.metric);
+            break;
+          case 'double':
+            Preferences.setDoublePref(this.pref, this.value,
+                                      !this.dialogPref, this.metric);
+            break;
+          case 'url':
+            Preferences.setURLPref(this.pref, this.value,
+                                   !this.dialogPref, this.metric);
+            break;
+          default:
+            Preferences.setStringPref(this.pref, this.value,
+                                      !this.dialogPref, this.metric);
+            break;
+        }
+      });
 
       window.addEventListener('unload',
           function() {
             if (document.activeElement == self)
               self.blur();
           });
     },
 
     /**
      * See |updateDisabledState_| above.
      */
     setDisabled: function(reason, disabled) {
       updateDisabledState_(this, reason, disabled);
     },
   };
 
   /**
    * The preference name.
    * @type {string}
    */
   cr.defineProperty(PrefTextField, 'pref', cr.PropertyKind.ATTR);
 
   /**
    * A special preference type specific to dialogs. These preferences are reset
-   * when the dialog is shown and are not saved until the user confirms the
+   * when the dialog is closed and are not saved until the user confirms the
    * dialog.
    * @type {boolean}
    */
   cr.defineProperty(PrefTextField, 'dialogPref', cr.PropertyKind.BOOL_ATTR);
 
   /**
    * Whether the preference is controlled by something else than the user's
    * settings (either 'policy' or 'extension').
    * @type {string}
    */
@@ skipping 71 matching lines @@
     PrefNumber: PrefNumber,
     PrefNumeric: PrefNumeric,
     PrefRadio: PrefRadio,
     PrefRange: PrefRange,
     PrefSelect: PrefSelect,
     PrefTextField: PrefTextField,
     PrefButton: PrefButton
   };
 
 });