[web-ui settings] Fixes and improvements for settings page searching.

 - Fixed a bug that prevented searching from the last page of the settings window.
 - Fixed appearance of the extensions page title.
 - Stop the extensions page from listening to NAV_ENTRY_COMMITTED, this event is no longer needed
   since the migration and was generating extension loads for every settings page change.
 - Prevent redundant searches from occurring when the user first starts typing into the search field.
 - Use custom 'searchChanged' events rather than key/mouse events for customizing extensions list
   appearance during a search.

BUG=99047
TEST=See reproduction steps in bug.

Review URL: http://codereview.chromium.org/8353022

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@107033 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/resources/options/extension_list.js

@@ -61,10 +61,9 @@ cr.define('options', function() {
 
       // Instal global event handlers.
       if (!handlersInstalled) {
-        this.ownerDocument.addEventListener('keyup',
-                                            this.upEventHandler_.bind(this));
-        this.ownerDocument.addEventListener('mouseup',
-                                            this.upEventHandler_.bind(this));
+        var searchPage = SearchPage.getInstance();
+        searchPage.addEventListener('searchChanged',
+                                    this.searchChangedHandler_.bind(this));
 
         // Support full keyboard accessibility without making things ugly
         // for users who click, by hiding some focus outlines when the user
@@ -648,18 +647,18 @@ cr.define('options', function() {
     },
 
     /**
-     * Handles the mouse-up and keyboard-up events. This is used to limit the
-     * number of items to show in the list, when the user is searching for items
-     * with the search box. Otherwise, if one match is found, the whole list of
+     * Handles the 'searchChanged' event. This is used to limit the number of
+     * items to show in the list, when the user is searching for items with the
+     * search box. Otherwise, if one match is found, the whole list of
      * extensions would be shown when we only want the matching items to be
      * found.
      * @param {Event} e Change event.
      * @private
      */
-    upEventHandler_: function(e) {
-      var searchString = $('search-field').value.toLowerCase();
+    searchChangedHandler_: function(e) {
+      var searchString = e.searchText;
       var child = this.firstChild;
-      while (child){
+      while (child) {
         var extension = this.getExtensionWithId_(child.id);
         if (searchString.length == 0) {
           // Show all.

chrome/browser/resources/options/extension_settings.html

 <div id="extension-settings" class="page" hidden>
-  <h1 i18n-content="extensionSettingsTitle"></h1>
+  <h1 i18n-content="extensionSettings"></h1>
   <div id="dev-toggle">
     <input id="toggle-dev-on" type="checkbox" value="off"></input>
     <label for="toggle-dev-on" i18n-content="extensionSettingsDeveloperMode" />

chrome/browser/resources/options/extension_settings.js

@@ -16,9 +16,8 @@ cr.define('options', function() {
    * @class
    */
   function ExtensionSettings() {
-    OptionsPage.call(this,
-                     'extensions',
-                     templateData.extensionSettingsTitle,
+    OptionsPage.call(this, 'extensions',
+                     templateData.extensionSettingsTabTitle,
                      'extension-settings');
   }
 

chrome/browser/resources/options/options_page.js

@@ -100,6 +100,7 @@ cr.define('options', function() {
     }
 
     pageName = targetPage.name.toLowerCase();
+    var targetPageWasVisible = targetPage.visible;
 
     // Determine if the root page is 'sticky', meaning that it
     // shouldn't change when showing a sub-page.  This can happen for special
@@ -117,14 +118,11 @@ cr.define('options', function() {
         page.willHidePage();
     }
 
-    var prevVisible = false;
-
     // Update visibilities to show only the hierarchy of the target page.
     for (var name in this.registeredPages) {
       var page = this.registeredPages[name];
       if (!page.parentPage && isRootPageLocked)
         continue;
-      prevVisible = page.visible;
       page.visible = name == pageName ||
           (!document.documentElement.classList.contains('hide-menu') &&
            page.isAncestorOfPage(targetPage));
@@ -142,7 +140,7 @@ cr.define('options', function() {
       var page = this.registeredPages[name];
       if (!page.parentPage && isRootPageLocked)
         continue;
-      if (!prevVisible && page.didShowPage && (name == pageName ||
+      if (!targetPageWasVisible && page.didShowPage && (name == pageName ||
           page.isAncestorOfPage(targetPage)))
         page.didShowPage();
     }

chrome/browser/resources/options/search_page.js

@@ -105,7 +105,6 @@ cr.define('options', function() {
   function SearchPage() {
     OptionsPage.call(this, 'search', templateData.searchPageTabTitle,
         'searchPage');
-    this.searchActive = false;
   }
 
   cr.addSingletonGetter(SearchPage);
@@ -114,6 +113,13 @@ cr.define('options', function() {
     // Inherit SearchPage from OptionsPage.
     __proto__: OptionsPage.prototype,
 
+    /**
+     * A boolean to prevent recursion. Used by setSearchText_().
+     * @type {Boolean}
+     * @private
+     */
+    insideSetSearchText_: false,
+
     /**
      * Initialize the page.
      */
@@ -145,7 +151,7 @@ cr.define('options', function() {
       // Handle search events. (No need to throttle, WebKit's search field
       // will do that automatically.)
       searchField.onsearch = function(e) {
-        self.setSearchText_(SearchPage.canonicalizeQuery(this.value));
+        self.setSearchText_(this.value);
       };
 
       // We update the history stack every time the search field blurs. This way
@@ -284,6 +290,19 @@ cr.define('options', function() {
      * @private
      */
     setSearchText_: function(text) {
+      // Prevent recursive execution of this method.
+      if (this.insideSetSearchText_) return;
+      this.insideSetSearchText_ = true;
+
+      // Cleanup the search query string.
+      text = SearchPage.canonicalizeQuery(text);
+
+      // Notify listeners about the new search query, some pages may wish to
+      // show/hide elements based on the query.
+      var event = new cr.Event('searchChanged');
+      event.searchText = text;
+      this.dispatchEvent(event);
+
       // Toggle the search page if necessary.
       if (text.length) {
         if (!this.searchActive_)
@@ -291,6 +310,8 @@ cr.define('options', function() {
       } else {
         if (this.searchActive_)
           OptionsPage.showDefaultPage();
+
+        this.insideSetSearchText_ = false;
         return;
       }
 
@@ -381,6 +402,9 @@ cr.define('options', function() {
       length = bubbleControls.length;
       for (var i = 0; i < length; i++)
         this.createSearchBubble_(bubbleControls[i], text);
+
+      // Cleanup the recursion-prevention variable.
+      this.insideSetSearchText_ = false;
     },
 
     /**

chrome/browser/ui/webui/options/extension_settings_handler.cc

@@ -217,9 +217,6 @@ void ExtensionSettingsHandler::MaybeRegisterForNotifications() {
                  content::Source<Profile>(profile));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_WARNING_CHANGED,
                  content::Source<Profile>(profile));
-  registrar_.Add(this,
-                 content::NOTIFICATION_NAV_ENTRY_COMMITTED,
-                 content::NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this,
                  content::NOTIFICATION_RENDER_VIEW_HOST_CREATED,
                  content::NotificationService::AllBrowserContextsAndSources());
@@ -505,10 +502,8 @@ void ExtensionSettingsHandler::GetLocalizedValues(
   DCHECK(localized_strings);
 
   RegisterTitle(localized_strings, "extensionSettings",
-                IDS_OPTIONS_GENERAL_TAB_LABEL);
+                IDS_MANAGE_EXTENSIONS_SETTING_WINDOWS_TITLE);
 
-  localized_strings->SetString("extensionSettingsTitle",
-      l10n_util::GetStringUTF16(IDS_MANAGE_EXTENSIONS_SETTING_WINDOWS_TITLE));
   localized_strings->SetString("extensionSettingsVisitWebsite",
       l10n_util::GetStringUTF16(IDS_EXTENSIONS_VISIT_WEBSITE));
 
@@ -605,12 +600,7 @@ void ExtensionSettingsHandler::Observe(
     // For instance, EXTENSION_LOADED & EXTENSION_PROCESS_CREATED because
     // we don't know about the views for an extension at EXTENSION_LOADED, but
     // if we only listen to EXTENSION_PROCESS_CREATED, we'll miss extensions
-    // that don't have a process at startup. Similarly, NAV_ENTRY_COMMITTED &
-    // RENDER_VIEW_HOST_CREATED because we want to handle both
-    // the case of navigating from a non-extension page to an extension page in
-    // a TabContents (which will generate NAV_ENTRY_COMMITTED) as well as
-    // extension content being shown in popups and balloons (which will generate
-    // RENDER_VIEW_HOST_CREATED but no NAV_ENTRY_COMMITTED).
+    // that don't have a process at startup.
     //
     // Doing it this way gets everything but causes the page to be rendered
     // more than we need. It doesn't seem to result in any noticeable flicker.
@@ -635,14 +625,6 @@ void ExtensionSettingsHandler::Observe(
           return;
       MaybeUpdateAfterNotification();
       break;
-    case content::NOTIFICATION_NAV_ENTRY_COMMITTED:
-      source_profile = Profile::FromBrowserContext(
-          content::Source<NavigationController>(
-              source).ptr()->browser_context());
-      if (!profile->IsSameProfile(source_profile))
-        return;
-      MaybeUpdateAfterNotification();
-      break;
     case chrome::NOTIFICATION_EXTENSION_LOADED:
     case chrome::NOTIFICATION_EXTENSION_PROCESS_CREATED:
     case chrome::NOTIFICATION_EXTENSION_UNLOADED: