Tidy up <select>s in tabbed options.

chrome/browser/resources/options/browser_options.js

Index: chrome/browser/resources/options/browser_options.js
diff --git a/chrome/browser/resources/options/browser_options.js b/chrome/browser/resources/options/browser_options.js
index 3b4198416e0bdd4735871d6157a249dfe76b52e5..a029f71c30beae4a58f8b0412d6bf6228f02f83b 100644
--- a/chrome/browser/resources/options/browser_options.js
+++ b/chrome/browser/resources/options/browser_options.js
@@ -62,6 +62,14 @@ cr.define('options', function() {
           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 =
@@ -351,12 +359,6 @@ cr.define('options', function() {
      * 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)]);
-      }
     },
   };