Tidy up <select>s in tabbed options.

chrome/browser/resources/options/browser_options.js

 // Copyright (c) 2010 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() {
   const OptionsPage = options.OptionsPage;
   const ArrayDataModel = cr.ui.ArrayDataModel;
   const ListSelectionModel = cr.ui.ListSelectionModel;
 
   //
@@ skipping 44 matching lines @@
       $('instantEnableCheckbox').onclick = function(event) {
         var alreadyConfirmed = $('instantDialogShown').checked;
 
         if (this.checked && !alreadyConfirmed) {
           // Leave disabled for now. The PrefCheckbox handler already set it to
           // true so undo that.
           Preferences.setBooleanPref(this.pref, false, this.metric);
           OptionsPage.showOverlay('instantConfirmOverlay');
         }
       };
+      $('defaultSearchEngine').onchange = function(event) {
+        var engineSelect = $('defaultSearchEngine');
+        var selectedIndex = engineSelect.selectedIndex;
+        if (selectedIndex >= 0) {
+          var selection = engineSelect.options[selectedIndex];
+          chrome.send('setDefaultSearchEngine', [String(selection.value)]);
+        }
+      };
 
       var homepageField = $('homepageURL');
       $('homepageUseNTPButton').onchange =
           this.handleHomepageUseNTPButtonChange_.bind(this);
       $('homepageUseURLButton').onchange =
           this.handleHomepageUseURLButtonChange_.bind(this);
       homepageField.onchange =
           this.handleHomepageURLChange_.bind(this);
 
       // Ensure that changes are committed when closing the page.
@@ skipping 268 matching lines @@
     updateCustomStartupPageControlStates_: function() {
       var disable = !this.shouldEnableCustomStartupPageControls_();
       $('startupPagesShortList').disabled = disable;
       $('startupUseCurrentButton').disabled = disable;
       $('startupPageManagerButton').disabled = disable;
     },
 
     /**
      * Set the default search engine based on the popup selection.
      */
     setDefaultSearchEngine: function() {

[stuartmorgan, 2011/01/12 22:59:41]

Why is this still here?

Or, alternately, why is it better to have this be an anonymous function instead
of binding onchange to this method? Having a named and documented function seems
like a win to me for readability, so I don't understand the point of this
change.

[Evan Stade, 2011/01/12 23:19:20]

well, it's not supposed to be... i will remove the stub.

I wanted to remove the onchange handler from the .html because it seems to me
that javascript doesn't belong there. In doing so I matched the style of the
other handlers above, which used anonymous functions. An additional win is that
the code is more self-documenting now and doesn't need other comments.

[stuartmorgan, 2011/01/12 23:30:09]

Well, I wouldn't have made the other non-trivial function there inline either.
If you bound to this.setDefaultSearchEngine in initializePage (which I agree is
better than having it in the HTML), then there's a function name that describes
the behavior. Inlining it removes the function name; I don't see how it can
possibly be more self-documenting when the only change is having less
human-readable information.

I guess if you feel strongly about this I'm okay with it, but it feels like
unnecessary thrashing on personal style.

-      var engineSelect = $('defaultSearchEngine');
-      var selectedIndex = engineSelect.selectedIndex;
-      if (selectedIndex >= 0) {
-        var selection = engineSelect.options[selectedIndex];
-        chrome.send('setDefaultSearchEngine', [String(selection.value)]);
-      }
     },
   };
 
   BrowserOptions.updateDefaultBrowserState = function(statusString, isDefault,
                                                       canBeDefault) {
     if (!cr.isChromeOS) {
       BrowserOptions.getInstance().updateDefaultBrowserState_(statusString,
                                                               isDefault,
                                                               canBeDefault);
     }
   };
 
   BrowserOptions.updateSearchEngines = function(engines, defaultValue) {
     BrowserOptions.getInstance().updateSearchEngines_(engines, defaultValue);
   };
 
   BrowserOptions.updateStartupPages = function(pages) {
     BrowserOptions.getInstance().updateStartupPages_(pages);
     StartupPageManager.getInstance().updateStartupPages_(pages);
   };
 
   // Export
   return {
     BrowserOptions: BrowserOptions
   };
 
 });