Fixes wrong inheritance of PrefSelect.

chrome/browser/resources/options/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.
 
 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
@@ skipping 372 matching lines @@
   };
 
   /////////////////////////////////////////////////////////////////////////////
   // 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__: PrefInputElement.prototype,
+    __proto__: HTMLSelectElement.prototype,

[Dan Beam, 2015/04/20 15:06:26]

keep in mind that settings is being rewritten actively

i think it's better to do one of two things:
1) make an abstract class that is concretely derived into
Pref{Input,Select}Element
2) make a concrete base class that has-an element (rather than is-an element)

both of these sound mildly messy and slightly error prone (but at least we have
typechecking now)

[arv (Not doing code reviews), 2015/04/24 14:22:45]

With ES6 you can do traits.

function PrefTrait(superClass) {
  return class extends superClass {
    handleChange(e) { ... }
  };
}

class PrefSelect extends PrefTrait(HTMLSelectElement) {
  ...
}

class PrefInputElement extends PrefTrait(HTMLInputElement) {
  ...
}

Of course you can desugar that to ES5 if you want.

[Yuki, 2015/04/27 09:22:38]

Looks cool to me.  I defer this re-design task to the maintainers (code owners).

+
+    /**
+     * Initialization function for the cr.ui framework.
+     */
+    decorate: PrefInputElement.prototype.decorate,
+
+    /**
+     * Handle changes to the input element's state made by the user. If a custom
+     * change handler does not suppress it, a default handler is invoked that
+     * updates the associated pref.
+     * @param {Event} event Change event.
+     * @protected
+     */
+    handleChange: PrefInputElement.prototype.handleChange,
 
     /**
      * Update the associated pref when when the user selects an item.
      * @override
      */
     updatePrefFromState: function() {
       var value = this.options[this.selectedIndex].value;
       switch (this.dataType) {
         case 'number':
           Preferences.setIntegerPref(this.pref, value,
@@ skipping 38 matching lines @@
       if (!found)
         this.selectedIndex = 0;
 
       // The "onchange" event automatically fires when the user makes a manual
       // change. It should never be fired for a programmatic change. However,
       // these two lines were here already and it is hard to tell who may be
       // relying on them.
       if (this.onchange)
         this.onchange(event);
     },
+
+    /**
+     * See |updateDisabledState| above.
+     */
+    setDisabled: PrefInputElement.prototype.setDisabled,
+
+    /**
+     * Custom change handler that is invoked first when the user makes changes

[arv (Not doing code reviews), 2015/04/24 14:22:45]

Are these comments just copied from PrefInputElement? If so remove them and add
something like "see PrefInputElement".

[Dan Beam, 2015/04/24 21:48:29]

or [just] @override

[Yuki, 2015/04/27 09:22:38]

Done.

+     * to the input element's state. If it returns false, a default handler is
+     * invoked next that updates the associated pref. If it returns true, the
+     * default handler is suppressed (i.e., this works like stopPropagation or
+     * cancelBubble).
+     * @param {Event} event Input element change event.
+     */
+    customChangeHandler: PrefInputElement.prototype.customChangeHandler,
+
+    /**
+     * Custom change handler that is invoked first when the preference
+     * associated with the input element changes. If it returns false, a default
+     * handler is invoked next that updates the input element. If it returns
+     * true, the default handler is suppressed.
+     * @param {Event} event Input element change event.
+     */
+    customPrefChangeHandler: PrefInputElement.prototype.customPrefChangeHandler,
   };
 
+  /**
+   * The name of the associated preference.
+   */
+  cr.defineProperty(PrefSelect, 'pref', cr.PropertyKind.ATTR);
+
+  /**
+   * The data type of the associated preference, only relevant for derived
+   * classes that support different data types.
+   */
+  cr.defineProperty(PrefSelect, 'dataType', cr.PropertyKind.ATTR);
+
+  /**
+   * Whether this input element is part of a dialog. If so, changes take effect
+   * in the settings UI immediately but are only actually committed when the
+   * user confirms the dialog. If the user cancels the dialog instead, the
+   * changes are rolled back in the settings UI and never committed.
+   */
+  cr.defineProperty(PrefSelect, 'dialogPref', cr.PropertyKind.BOOL_ATTR);
+
+  /**
+   * Whether the associated preference is controlled by a source other than the
+   * user's setting (can be 'policy', 'extension', 'recommended' or unset).
+   */
+  cr.defineProperty(PrefSelect, 'controlledBy', cr.PropertyKind.ATTR);
+
+  /**
+   * The user metric string.
+   */
+  cr.defineProperty(PrefSelect, 'metric', 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__: PrefInputElement.prototype,
 
@@ skipping 121 matching lines @@
     PrefInputElement: PrefInputElement,
     PrefNumber: PrefNumber,
     PrefRadio: PrefRadio,
     PrefRange: PrefRange,
     PrefSelect: PrefSelect,
     PrefTextField: PrefTextField,
     PrefPortNumber: PrefPortNumber,
     PrefButton: PrefButton
   };
 });