DOMUI Prefs: Split search engines into two lists

BUG=70332
TEST=Search engine list headers should match autofill options.

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

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

chrome/browser/dom_ui/options/search_engine_manager_handler.cc

@@ -50,6 +50,10 @@ void SearchEngineManagerHandler::GetLocalizedValues(
 
   localized_strings->SetString("searchEngineManagerPage",
       l10n_util::GetStringUTF16(IDS_SEARCH_ENGINES_EDITOR_WINDOW_TITLE));
+  localized_strings->SetString("defaultSearchEngineListTitle",
+      l10n_util::GetStringUTF16(IDS_SEARCH_ENGINES_EDITOR_MAIN_SEPARATOR));
+  localized_strings->SetString("otherSearchEngineListTitle",
+      l10n_util::GetStringUTF16(IDS_SEARCH_ENGINES_EDITOR_OTHER_SEPARATOR));
   localized_strings->SetString("searchEngineTableNameHeader",
       l10n_util::GetStringUTF16(IDS_SEARCH_ENGINES_EDITOR_DESCRIPTION_COLUMN));
   localized_strings->SetString("searchEngineTableKeywordHeader",
@@ -98,33 +102,31 @@ void SearchEngineManagerHandler::OnModelChanged() {
   if (!list_controller_->loaded())
     return;
 
-  ListValue engine_list;
-
   // Find the default engine.
   const TemplateURL* default_engine =
       list_controller_->url_model()->GetDefaultSearchProvider();
   int default_index = list_controller_->table_model()->IndexOfTemplateURL(
       default_engine);
 
-  // Add the first group (default search engine options).
-  engine_list.Append(CreateDictionaryForHeading(0));
+  // Build the first list (default search engine options).
+  ListValue defaults_list;
   int last_default_engine_index =
       list_controller_->table_model()->last_search_engine_index();
   for (int i = 0; i < last_default_engine_index; ++i) {
-    engine_list.Append(CreateDictionaryForEngine(i, i == default_index));
+    defaults_list.Append(CreateDictionaryForEngine(i, i == default_index));
   }
 
-  // Add the second group (other search templates).
-  engine_list.Append(CreateDictionaryForHeading(1));
+  // Build the second list (other search templates).
+  ListValue others_list;
   if (last_default_engine_index < 0)
     last_default_engine_index = 0;
   int engine_count = list_controller_->table_model()->RowCount();
   for (int i = last_default_engine_index; i < engine_count; ++i) {
-    engine_list.Append(CreateDictionaryForEngine(i, i == default_index));
+    others_list.Append(CreateDictionaryForEngine(i, i == default_index));
   }
 
   dom_ui_->CallJavascriptFunction(L"SearchEngineManager.updateSearchEngineList",
-                                  engine_list);
+                                  defaults_list, others_list);
 }
 
 void SearchEngineManagerHandler::OnItemsChanged(int start, int length) {
@@ -139,15 +141,6 @@ void SearchEngineManagerHandler::OnItemsRemoved(int start, int length) {
   OnModelChanged();
 }
 
-DictionaryValue* SearchEngineManagerHandler::CreateDictionaryForHeading(
-    int group_index) {
-  ui::TableModel::Groups groups = list_controller_->table_model()->GetGroups();
-
-  DictionaryValue* dict = new DictionaryValue();
-  dict->SetString("heading", groups[group_index].title);
-  return dict;
-}
-
 DictionaryValue* SearchEngineManagerHandler::CreateDictionaryForEngine(
     int index, bool is_default) {
   TemplateURLTableModel* table_model = list_controller_->table_model();

chrome/browser/dom_ui/options/search_engine_manager_handler.h

@@ -64,8 +64,6 @@ class SearchEngineManagerHandler : public OptionsPageUIHandler,
   // Called from DOMUI.
   void EditCompleted(const ListValue* args);
 
-  // Returns a dictionary to pass to DOMUI representing the given group heading.
-  DictionaryValue* CreateDictionaryForHeading(int group_index);
   // Returns a dictionary to pass to DOMUI representing the given search engine.
   DictionaryValue* CreateDictionaryForEngine(int index, bool is_default);
 

chrome/browser/resources/options/list_inline_header_selection_controller.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 ListSelectionController = cr.ui.ListSelectionController;
-
-  /**
-   * Creates a selection controller with a delegate that controls whether or
-   * not individual items are selectable. This is used for lists containing
-   * subgroups with headings that are in the list, since the headers themselves
-   * should not be selectable.
-   *
-   * @param {cr.ui.ListSelectionModel} selectionModel The selection model to
-   *     interact with.
-   * @param {*} selectabilityDelegate A delegate responding to
-   *     canSelectIndex(index).
-   *
-   * @constructor
-   * @extends {!cr.ui.ListSelectionController}
-   */
-  function ListInlineHeaderSelectionController(selectionModel,
-                                               selectabilityDelegate) {
-    ListSelectionController.call(this, selectionModel);
-    this.selectabilityDelegate_ = selectabilityDelegate;
-  }
-
-  ListInlineHeaderSelectionController.prototype = {
-    __proto__: ListSelectionController.prototype,
-
-    /** @inheritDoc */
-    getIndexBelow: function(index) {
-      var next = ListSelectionController.prototype.getIndexBelow.call(this,
-                                                                      index);
-      if (next == -1 || this.canSelect(next))
-        return next;
-      return this.getIndexBelow(next);
-    },
-
-    /** @inheritDoc */
-    getNextIndex: function(index) {
-      return this.getIndexBelow(index);
-    },
-
-    /** @inheritDoc */
-    getIndexAbove: function(index) {
-      var previous = ListSelectionController.prototype.getIndexAbove.call(
-          this, index);
-      if (previous == -1 || this.canSelect(previous))
-        return previous;
-      return this.getIndexAbove(previous);
-    },
-
-    /** @inheritDoc */
-    getPreviousIndex: function(index) {
-      return this.getIndexAbove(index);
-    },
-
-    /** @inheritDoc */
-    getFirstIndex: function(index) {
-      var first = ListSelectionController.prototype.getFirstIndex.call(this);
-      if (this.canSelect(first))
-        return first;
-      return this.getNextIndex(first);
-    },
-
-    /** @inheritDoc */
-    getLastIndex: function(index) {
-      var last = ListSelectionController.prototype.getLastIndex.call(this);
-      if (this.canSelect(last))
-        return last;
-      return this.getPreviousIndex(last);
-    },
-
-    /** @inheritDoc */
-    handleMouseDownUp: function(e, index) {
-      if (this.canSelect(index)) {
-        ListSelectionController.prototype.handleMouseDownUp.call(
-            this, e, index);
-      }
-    },
-
-    /**
-     * Returns true if the given index is selectable.
-     * @private
-     * @param {number} index The index to check.
-     */
-    canSelect: function(index) {
-      return this.selectabilityDelegate_.canSelectIndex(index);
-    }
-  };
-
-  return {
-    ListInlineHeaderSelectionController: ListInlineHeaderSelectionController
-  };
-});

chrome/browser/resources/options/options.html

@@ -61,7 +61,6 @@
 <script src="pref_ui.js"></script>
 <script src="deletable_item_list.js"></script>
 <script src="inline_editable_list.js"></script>
-<script src="list_inline_header_selection_controller.js"></script>
 <script src="options_page.js"></script>
 <if expr="pp_ifdef('chromeos')">
   <script src="about_page.js"></script>

chrome/browser/resources/options/search_engine_manager.css

-#searchEngineList {
-  -webkit-border-radius: 2px;
-  border-bottom: 1px solid #d9d9d9;
-  height: auto;
-}
-
-#searchEngineList .heading {
-  -webkit-border-radius: 2px;
-  color: black;
-  border-bottom: 1px solid #d9d9d9;
-}
-
-#searchEngineList .heading:not(:nth-child(2)) {
-  border-top: 1px solid #d9d9d9;
-}
-
-#searchEngineList .heading .name-column {
-  font-weight: bold;
-}
-
-#searchEngineList > div:not(.heading) {
-  border-left: 1px solid #d9d9d9;
-  border-right: 1px solid #d9d9d9;
-}
-
-#searchEngineList > div, #searchEngineHeading {
+.search-engine-list > div {
   display: -webkit-box;
 }
 
-#searchEngineList .favicon {
+.search-engine-list .favicon {
   padding: 1px 7px 0px 7px;
   height: 16px;
 }
 
-#searchEngineList .name-column {
+.search-engine-list .name-column {
+  -webkit-padding-end: 1ex;
   box-sizing: border-box;
   width: 37%;
 }
 
-#searchEngineList .keyword-column {
+.search-engine-list .keyword-column {
   width: 26%;
 }
 
-#searchEngineList .url-column {
+.search-engine-list .url-column {
   width: 37%;
 }
 
-#searchEngineList > div:not(.heading) .keyword-column,
-#searchEngineList > div:not(.heading) .url-column {
-  color: #666666;
-}
-
-#searchEngineList .name-column,
-#searchEngineList .keyword-column,
-#searchEngineList .url-column {
+.search-engine-list .keyword-column,
+.search-engine-list .url-column {
   -webkit-padding-end: 1ex;
+  color: #666666;
 }
 
-#searchEngineList .default {
+.search-engine-list .default .name-column,
+.search-engine-list .default .keyword-column {
   font-weight: bold;
 }
 
-#searchEngineList .default .url-column {
-  font-weight: normal;
-}
-
-#searchEngineList .name-column {
+.search-engine-list .name-column {
   display: -webkit-box;
 }
 
-#searchEngineList .name-column :last-child {
+.search-engine-list .name-column :last-child {
   -webkit-box-flex: 1;
 }
 
-#searchEngineList input {
+.search-engine-list input {
   box-sizing: border-box;
   margin: 0;
   width: 100%;
 }
 
 /* For temporary Make Default button */
-#searchEngineList .url-column {
+.search-engine-list .url-column {
   display: -webkit-box;
   -webkit-box-align: center;
 }
 
-#searchEngineList .url-column :first-child {
+.search-engine-list .url-column :first-child {
   -webkit-box-flex: 1;
 }
 
-#searchEngineList .url-column button {
+.search-engine-list .url-column button {
   background: #8aaaed;
   color: #fff;
   -webkit-margin-start: 3px;
 }
 
-#searchEngineList > :not(:hover):not([editing]) .url-column button {
+.search-engine-list > :not(:hover):not([editing]) .url-column button {
   display: none;
 }
 /* End temporary Make Default button styling */

chrome/browser/resources/options/search_engine_manager.html

 <div class="page hidden" id="searchEngineManagerPage">
   <h1 i18n-content="searchEngineManagerPage"></h1>
-  <list id="searchEngineList"></list>
+  <h3 i18n-content="defaultSearchEngineListTitle"></h3>
+  <list id="defaultSearchEngineList"
+      class="search-engine-list settings-list"></list>
+  <h3 i18n-content="otherSearchEngineListTitle"></h3>
+  <list id="otherSearchEngineList"
+      class=" search-engine-list settings-list"></list>
 </div>

chrome/browser/resources/options/search_engine_manager.js

@@ -22,39 +22,73 @@ cr.define('options', function() {
 
   SearchEngineManager.prototype = {
     __proto__: OptionsPage.prototype,
-    list_: null,
 
+    /**
+     * List for default search engine options
+     * @type {boolean}
+     * @private
+     */
+    defaultsList_: null,
+
+    /**
+     * List for other search engine options
+     * @type {boolean}
+     * @private
+     */
+    othersList_: null,
+
+    /** inheritDoc */
     initializePage: function() {
       OptionsPage.prototype.initializePage.call(this);
 
-      this.list_ = $('searchEngineList')
-      options.search_engines.SearchEngineList.decorate(this.list_);
-      var selectionModel = new ListSingleSelectionModel;
-      this.list_.selectionModel = selectionModel;
-      this.list_.autoExpands = true;
+      this.defaultsList_ = $('defaultSearchEngineList');
+      this.setUpList_(this.defaultsList_);
+
+      this.othersList_ = $('otherSearchEngineList');
+      this.setUpList_(this.othersList_);
+    },
+
+    /**
+     * Sets up the given list as a search engine list
+     * @param {List} list The list to set up.
+     * @private
+     */
+    setUpList_: function(list) {
+      options.search_engines.SearchEngineList.decorate(list);
+      list.selectionModel = new ListSingleSelectionModel;
+      list.autoExpands = true;
     },
 
     /**
      * Updates the search engine list with the given entries.
      * @private
-     * @param {Array} engineList List of available search engines.
+     * @param {Array} defaultEngines List of possible default search engines.
+     * @param {Array} otherEngines List of other search engines.
      */
-    updateSearchEngineList_: function(engineList) {
-      var model = new ArrayDataModel(engineList);
-      model.push({
+    updateSearchEngineList_: function(defaultEngines, otherEngines) {
+      this.defaultsList_.dataModel = new ArrayDataModel(defaultEngines);
+      var othersModel = new ArrayDataModel(otherEngines);
+      // Add a "new engine" row.
+      othersModel.push({
         'modelIndex': '-1'
       });
-      this.list_.dataModel = model;
+      this.othersList_.dataModel = othersModel;
     },
   };
 
-  SearchEngineManager.updateSearchEngineList = function(engineList) {
-    SearchEngineManager.getInstance().updateSearchEngineList_(engineList);
+  SearchEngineManager.updateSearchEngineList = function(defaultEngines,
+                                                        otherEngines) {
+    SearchEngineManager.getInstance().updateSearchEngineList_(defaultEngines,
+                                                              otherEngines);
   };
 
   SearchEngineManager.validityCheckCallback = function(validity, modelIndex) {
-    SearchEngineManager.getInstance().list_.validationComplete(validity,
-                                                               modelIndex);
+    // Forward to both lists; the one without a matching modelIndex will ignore
+    // it.
+    SearchEngineManager.getInstance().defaultsList_.validationComplete(
+        validity, modelIndex);
+    SearchEngineManager.getInstance().othersList_.validationComplete(
+        validity, modelIndex);
   };
 
   // Export

chrome/browser/resources/options/search_engine_manager_engine_list.js

@@ -5,8 +5,7 @@
 cr.define('options.search_engines', function() {
   const InlineEditableItemList = options.InlineEditableItemList;
   const InlineEditableItem = options.InlineEditableItem;
-  const ListInlineHeaderSelectionController =
-      options.ListInlineHeaderSelectionController;
+  const ListSelectionController = cr.ui.ListSelectionController;
 
   /**
    * Creates a new search engine list item.
@@ -90,50 +89,37 @@ cr.define('options.search_engines', function() {
 
       this.currentlyValid_ = !this.isPlaceholder_;
 
-      if (engine['heading']) {
-        this.classList.add('heading');
-        this.editable = false;
-      } else if (engine['default']) {
+      if (engine['default'])
         this.classList.add('default');
-      }
 
       this.deletable = engine['canBeRemoved'];
 
-      var nameText = engine['name'];
-      var keywordText = engine['keyword'];
-      var urlText = engine['url'];
-      if (engine['heading']) {
-        nameText = engine['heading'];
-        keywordText = localStrings.getString('searchEngineTableKeywordHeader');
-        urlText = localStrings.getString('searchEngineTableURLHeader');
-      }
-
       // Construct the name column.
       var nameColEl = this.ownerDocument.createElement('div');
       nameColEl.className = 'name-column';
       this.contentElement.appendChild(nameColEl);
 
-      // For non-heading rows, start with a favicon.
-      if (!engine['heading']) {
-        var faviconDivEl = this.ownerDocument.createElement('div');
-        faviconDivEl.className = 'favicon';
-        var imgEl = this.ownerDocument.createElement('img');
-        imgEl.src = 'chrome://favicon/iconurl/' + engine['iconURL'];
-        faviconDivEl.appendChild(imgEl);
-        nameColEl.appendChild(faviconDivEl);
-      }
+      // Add the favicon.
+      var faviconDivEl = this.ownerDocument.createElement('div');
+      faviconDivEl.className = 'favicon';
+      var imgEl = this.ownerDocument.createElement('img');
+      imgEl.src = 'chrome://favicon/iconurl/' + engine['iconURL'];
+      faviconDivEl.appendChild(imgEl);
+      nameColEl.appendChild(faviconDivEl);
 
-      var nameEl = this.createEditableTextCell(nameText, this.isPlaceholder_);
+      var nameEl = this.createEditableTextCell(engine['name'],
+                                               this.isPlaceholder_);
       nameColEl.appendChild(nameEl);
 
       // Then the keyword column.
-      var keywordEl = this.createEditableTextCell(keywordText,
+      var keywordEl = this.createEditableTextCell(engine['keyword'],
                                                   this.isPlaceholder_);
       keywordEl.className = 'keyword-column';
       this.contentElement.appendChild(keywordEl);
 
       // And the URL column.
-      var urlEl = this.createEditableTextCell(urlText, this.isPlaceholder_);
+      var urlEl = this.createEditableTextCell(engine['url'],
+                                              this.isPlaceholder_);
       var urlWithButtonEl = this.ownerDocument.createElement('div');
       urlWithButtonEl.appendChild(urlEl);
       urlWithButtonEl.className = 'url-column';
@@ -142,6 +128,7 @@ cr.define('options.search_engines', function() {
       // is implemented. When this is removed, remove the extra div above.
       if (engine['canBeDefault']) {
         var makeDefaultButtonEl = this.ownerDocument.createElement('button');
+        makeDefaultButtonEl.className = "raw-button";
         makeDefaultButtonEl.textContent =
             templateData.makeDefaultSearchEngineButton;
         makeDefaultButtonEl.onclick = function(e) {
@@ -155,27 +142,25 @@ cr.define('options.search_engines', function() {
       }
 
       // Do final adjustment to the input fields.
-      if (!engine['heading']) {
-        this.nameField_ = nameEl.querySelector('input');
-        this.keywordField_ = keywordEl.querySelector('input');
-        this.urlField_ = urlEl.querySelector('input');
-
-        if (engine['urlLocked'])
-          this.urlField_.disabled = true;
-
-        if (this.isPlaceholder_) {
-          this.nameField_.placeholder =
-              localStrings.getString('searchEngineTableNamePlaceholder');
-          this.keywordField_.placeholder =
-              localStrings.getString('searchEngineTableKeywordPlaceholder');
-          this.urlField_.placeholder =
-              localStrings.getString('searchEngineTableURLPlaceholder');
-        }
-
-        var fields = [ this.nameField_, this.keywordField_, this.urlField_ ];
-          for (var i = 0; i < fields.length; i++) {
-          fields[i].oninput = this.startFieldValidation_.bind(this);
-        }
+      this.nameField_ = nameEl.querySelector('input');
+      this.keywordField_ = keywordEl.querySelector('input');
+      this.urlField_ = urlEl.querySelector('input');
+
+      if (engine['urlLocked'])
+        this.urlField_.disabled = true;
+
+      if (this.isPlaceholder_) {
+        this.nameField_.placeholder =
+            localStrings.getString('searchEngineTableNamePlaceholder');
+        this.keywordField_.placeholder =
+            localStrings.getString('searchEngineTableKeywordPlaceholder');
+        this.urlField_.placeholder =
+            localStrings.getString('searchEngineTableURLPlaceholder');
+      }
+
+      var fields = [ this.nameField_, this.keywordField_, this.urlField_ ];
+        for (var i = 0; i < fields.length; i++) {
+        fields[i].oninput = this.startFieldValidation_.bind(this);
       }
 
       // Listen for edit events.
@@ -302,25 +287,12 @@ cr.define('options.search_engines', function() {
       return new SearchEngineListItem(searchEngine);
     },
 
-    /** @inheritDoc */
-    createSelectionController: function(sm) {
-      return new ListInlineHeaderSelectionController(sm, this);
-    },
-
     /** @inheritDoc */
     deleteItemAtIndex: function(index) {
       var modelIndex = this.dataModel.item(index)['modelIndex']
       chrome.send('removeSearchEngine', [String(modelIndex)]);
     },
 
-    /**
-     * Returns true if the given item is selectable.
-     * @param {number} index The index to check.
-     */
-    canSelectIndex: function(index) {
-      return !this.dataModel.item(index).hasOwnProperty('heading');
-    },
-
     /**
      * Passes the results of an input validation check to the requesting row
      * if it's still being edited.
@@ -330,6 +302,8 @@ cr.define('options.search_engines', function() {
     validationComplete: function(validity, modelIndex) {
       // If it's not still being edited, it no longer matters.
       var currentSelection = this.selectedItem;
+      if (!currentSelection)
+        return;
       var listItem = this.getListItem(currentSelection);
       if (listItem.editing && currentSelection['modelIndex'] == modelIndex)
         listItem.validationComplete(validity);