DOMUI Prefs: Implement inline editability for startup pages

Also moves more common code into InlineEditableItem, and fixes some styling issues with inline-editable lists.

BUG=63817
TEST=Startup pages should be editable in DOMUI prefs. Also, the lead selection in an editable list should show top and bottom borders.

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

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

chrome/browser/custom_home_pages_table_model.cc

@@ -70,6 +70,7 @@ void CustomHomePagesTableModel::Add(int index, const GURL& url) {
   DCHECK(index >= 0 && index <= RowCount());
   entries_.insert(entries_.begin() + static_cast<size_t>(index), Entry());
   entries_[index].url = url;
+  LoadTitleAndFavIcon(&(entries_[index]));
   if (observer_)
     observer_->OnItemsAdded(index, 1);
 }

chrome/browser/dom_ui/options/browser_options_handler.cc

@@ -109,6 +109,9 @@ void BrowserOptionsHandler::RegisterMessages() {
   dom_ui_->RegisterMessageCallback(
       "addStartupPage",
       NewCallback(this, &BrowserOptionsHandler::AddStartupPage));
+  dom_ui_->RegisterMessageCallback(
+      "editStartupPage",
+      NewCallback(this, &BrowserOptionsHandler::EditStartupPage));
   dom_ui_->RegisterMessageCallback(
       "setStartupPagesToCurrentPages",
       NewCallback(this, &BrowserOptionsHandler::SetStartupPagesToCurrentPages));
@@ -285,6 +288,7 @@ void BrowserOptionsHandler::OnModelChanged() {
     entry->SetString("url", urls[i].spec());
     entry->SetString("tooltip",
                      startup_custom_pages_table_model_->GetTooltip(i));
+    entry->SetString("modelIndex", base::IntToString(i));
     startup_pages.Append(entry);
   }
 
@@ -314,8 +318,7 @@ void BrowserOptionsHandler::RemoveStartupPages(const ListValue* args) {
   for (int i = args->GetSize() - 1; i >= 0; --i) {
     std::string string_value;
     if (!args->GetString(i, &string_value)) {
-      NOTREACHED();
-      return;
+      CHECK(false);
     }
     int selected_index;
     base::StringToInt(string_value, &selected_index);
@@ -338,8 +341,7 @@ void BrowserOptionsHandler::AddStartupPage(const ListValue* args) {
       !args->GetString(0, &url_string) ||
       !args->GetString(1, &index_string) ||
       !base::StringToInt(index_string, &index)) {
-    NOTREACHED();
-    return;
+    CHECK(false);
   };
 
   if (index == -1)
@@ -353,6 +355,27 @@ void BrowserOptionsHandler::AddStartupPage(const ListValue* args) {
   SaveStartupPagesPref();
 }
 
+void BrowserOptionsHandler::EditStartupPage(const ListValue* args) {
+  std::string url_string;
+  std::string index_string;
+  int index;
+  if (args->GetSize() != 2 ||
+      !args->GetString(0, &index_string) ||
+      !base::StringToInt(index_string, &index) ||
+      !args->GetString(1, &url_string)) {
+    CHECK(false);
+  };
+
+  if (index < 0 || index > startup_custom_pages_table_model_->RowCount()) {
+    NOTREACHED();
+    return;
+  }
+
+  std::vector<GURL> urls = startup_custom_pages_table_model_->GetURLs();
+  urls[index] = URLFixerUpper::FixupURL(url_string, std::string());
+  startup_custom_pages_table_model_->SetURLs(urls);
+}
+
 void BrowserOptionsHandler::SaveStartupPagesPref() {
   PrefService* prefs = dom_ui_->GetProfile()->GetPrefs();
 

chrome/browser/dom_ui/options/browser_options_handler.h

@@ -57,6 +57,10 @@ class BrowserOptionsHandler : public OptionsPageUIHandler,
   // Called from DOMUI.
   void AddStartupPage(const ListValue* args);
 
+  // Changes the startup page at the given index to the given URL.
+  // Called from DOMUI.
+  void EditStartupPage(const ListValue* args);
+
   // Sets the startup page set to the current pages. Called from DOMUI.
   void SetStartupPagesToCurrentPages(const ListValue* args);
 

chrome/browser/resources/options/browser_options.html

@@ -18,7 +18,7 @@
               i18n-content="startupShowPages"></span></label>
       <div class="suboption">
         <div id="startupPageManagement" class="settings-list">
-          <list id="startupPagesShortList"></list>
+          <list id="startupPagesShortList" class="startup-page-list"></list>
           <div>
             <button id="startupPageManagerButton"
                 i18n-content="startupManagePages"></button>

chrome/browser/resources/options/browser_options.js

@@ -5,7 +5,7 @@
 cr.define('options', function() {
   const OptionsPage = options.OptionsPage;
   const ArrayDataModel = cr.ui.ArrayDataModel;
-  const ListSelectionModel = cr.ui.ListSelectionModel;
+  const ListSingleSelectionModel = cr.ui.ListSingleSelectionModel;
 
   //
   // BrowserOptions class
@@ -87,7 +87,7 @@ cr.define('options', function() {
       var list = $('startupPagesShortList');
       options.browser_options.StartupPageList.decorate(list);
       list.autoExpands = true;
-      list.selectionModel = new ListSelectionModel;
+      list.selectionModel = new ListSingleSelectionModel;
 
       // Check if we are in the guest mode.
       if (cr.commandLine.options['--bwsi']) {

chrome/browser/resources/options/browser_options_page.css

@@ -4,6 +4,21 @@
   padding: 5px 10px;
 }
 
+.startup-page-list .title {
+  width: 40%;
+}
+
+.startup-page-list .url {
+  -webkit-box-flex: 1;
+  color: #666;
+}
+
+.startup-page-list .url input {
+  box-sizing: border-box;
+  margin: 0;
+  width: 100%;
+}
+
 #customHomePageGroup {
   display: -webkit-box;
   -webkit-box-orient: horizontal;

chrome/browser/resources/options/browser_options_startup_page_list.js

@@ -3,8 +3,8 @@
 // found in the LICENSE file.
 
 cr.define('options.browser_options', function() {
-  const DeletableItemList = options.DeletableItemList;
-  const DeletableItem = options.DeletableItem;
+  const InlineEditableItem = options.InlineEditableItem;
+  const InlineEditableItemList = options.InlineEditableItemList;
 
   /**
    * Creates a new startup page list item.
@@ -29,13 +29,20 @@ cr.define('options.browser_options', function() {
   };
 
   StartupPageListItem.prototype = {
-    __proto__: DeletableItem.prototype,
+    __proto__: InlineEditableItem.prototype,
+
+    /**
+     * Input field for editing the page url.
+     * @type {HTMLElement}
+     * @private
+     */
+    urlField_: null,
 
     /** @inheritDoc */
     decorate: function() {
-      DeletableItem.prototype.decorate.call(this);
+      InlineEditableItem.prototype.decorate.call(this);
 
-      var titleEl = this.ownerDocument.createElement('span');
+      var titleEl = this.ownerDocument.createElement('div');
       titleEl.className = 'title';
       titleEl.classList.add('favicon-cell');
       titleEl.textContent = this.pageInfo_['title'];
@@ -44,13 +51,42 @@ cr.define('options.browser_options', function() {
       titleEl.title = this.pageInfo_['tooltip'];
 
       this.contentElement.appendChild(titleEl);
+
+      var urlEl = this.createEditableTextCell(this.pageInfo_['url']);
+      urlEl.className = 'url';
+      this.contentElement.appendChild(urlEl);
+
+      this.urlField_ = urlEl.querySelector('input');
+      this.urlField_.required = true;
+
+      this.addEventListener('commitedit', this.onEditCommitted_.bind(this));
+    },
+
+    /** @inheritDoc */
+    get currentInputIsValid() {
+      return this.urlField_.validity.valid;
+    },
+
+    /** @inheritDoc */
+    get hasBeenEdited() {
+      return this.urlField_.value != this.pageInfo_['url'];
+    },
+
+    /**
+     * Called when committing an edit; updates the model.
+     * @param {Event} e The end event.
+     * @private
+     */
+    onEditCommitted_: function(e) {
+      chrome.send('editStartupPage',
+                  [this.pageInfo_['modelIndex'], this.urlField_.value]);
     },
   };
 
   var StartupPageList = cr.ui.define('list');
 
   StartupPageList.prototype = {
-    __proto__: DeletableItemList.prototype,
+    __proto__: InlineEditableItemList.prototype,
 
     /** @inheritDoc */
     createItem: function(pageInfo) {

chrome/browser/resources/options/content_settings.css

@@ -56,7 +56,3 @@ html[toolkit=gtk] select.exception-setting {
   margin-bottom: 10px;
   margin-top: 4px;
 }
-
-#content-settings-exceptions-area input:invalid {
-  background-color: pink;
-}

chrome/browser/resources/options/inline_editable_list.js

@@ -90,10 +90,12 @@ cr.define('options', function() {
           }, 50);
         }
       } else {
-        if (!this.editCancelled_ && this.hasBeenEdited() &&
+        if (!this.editCancelled_ && this.hasBeenEdited &&
             this.currentInputIsValid) {
+          this.updateStaticValues_();
           cr.dispatchSimpleEvent(this, 'commitedit', true);
         } else {
+          this.resetEditableValues_();
           cr.dispatchSimpleEvent(this, 'canceledit', true);
         }
       }
@@ -114,11 +116,12 @@ cr.define('options', function() {
 
     /**
      * The HTML element that should have focus initially when editing starts.
-     * Should be overriden by subclasses.
+     * Defaults to the first <input> element; can be overriden by subclasses if
+     * a different element should be focused.
      * @type {HTMLElement}
      */
     get initialFocusElement() {
-      return null;
+      return this.contentElement.querySelector('input');
     },
 
     /**
@@ -126,6 +129,7 @@ cr.define('options', function() {
      * when editing would be submitted, either editing will not be ended,
      * or it will be cancelled, depending on the context.
      * Can be overrided by subclasses to perform input validation.
+     * @type {boolean}
      */
     get currentInputIsValid() {
       return true;
@@ -135,11 +139,80 @@ cr.define('options', function() {
      * Returns true if the item has been changed by an edit.
      * Can be overrided by subclasses to return false when nothing has changed
      * to avoid unnecessary commits.
+     * @type {boolean}
      */
-    hasBeenEdited: function() {
+    get hasBeenEdited() {
       return true;
     },
 
+    /**
+     * Returns a div containing an <input>, as well as static text if
+     * opt_alwaysEditable is not true.
+     * @param {string} text The text of the cell.
+     * @param {bool} opt_alwaysEditable True if the cell always shows the input.
+     * @return {HTMLElement} The HTML element for the cell.
+     * @private
+     */
+    createEditableTextCell: function(text, opt_alwaysEditable) {
+      var container = this.ownerDocument.createElement('div');
+
+      if (!opt_alwaysEditable) {
+        var textEl = this.ownerDocument.createElement('div');
+        textEl.className = 'static-text';
+        textEl.textContent = text;
+        textEl.setAttribute('editmode', false);
+        container.appendChild(textEl);
+      }
+
+      var inputEl = this.ownerDocument.createElement('input');
+      inputEl.type = 'text';
+      inputEl.value = text;
+      if (!opt_alwaysEditable) {
+        inputEl.setAttribute('editmode', true);
+        inputEl.staticVersion = textEl;
+      }
+      container.appendChild(inputEl);
+
+      return container;
+    },
+
+    /**
+     * Resets the editable version of any controls created by createEditable*
+     * to match the static text.
+     * @private
+     */
+    resetEditableValues_: function() {
+      var editFields = this.querySelectorAll('[editmode=true]');
+      for (var i = 0; i < editFields.length; i++) {
+        var staticLabel = editFields[i].staticVersion;
+        if (!staticLabel)
+          continue;
+        if (editFields[i].tagName == 'INPUT')
+          editFields[i].value = staticLabel.textContent;
+        // Add more tag types here as new createEditable* methods are added.
+
+        editFields[i].setCustomValidity('');
+      }
+    },
+
+    /**
+     * Sets the static version of any controls created by createEditable*
+     * to match the current value of the editable version. Called on commit so
+     * that there's no flicker of the old value before the model updates.
+     * @private
+     */
+    updateStaticValues_: function() {
+      var editFields = this.querySelectorAll('[editmode=true]');
+      for (var i = 0; i < editFields.length; i++) {
+        var staticLabel = editFields[i].staticVersion;
+        if (!staticLabel)
+          continue;
+        if (editFields[i].tagName == 'INPUT')
+          staticLabel.textContent = editFields[i].value;
+        // Add more tag types here as new createEditable* methods are added.
+      }
+    },
+
     /**
      * Called a key is pressed. Handles committing and cancelling edits.
      * @param {Event} e The key down event.

chrome/browser/resources/options/options_page.css

@@ -379,18 +379,21 @@ list > [selected]:hover {
 }
 
 list:focus > [lead],
-list:focus > [lead][selected] {
+list:focus > [lead][selected],
+list [lead][selected][editing] {
   border-top: 1px solid #b2b2b2;
   border-bottom: 1px solid #b2b2b2;
 }
 
 list:focus > [lead]:nth-child(2),
-list:focus > [lead][selected]:nth-child(2) {
+list:focus > [lead][selected]:nth-child(2),
+list [lead][selected][editing]:nth-child(2) {
   border-top: 1px solid #f2f2f2;
 }
 
 list:focus > [lead]:nth-last-child(2),
-list:focus > [lead][selected]:nth-last-child(2) {
+list:focus > [lead][selected]:nth-last-child(2),
+list [lead][selected][editing]:nth-last-child(2) {
   border-bottom: 1px solid #f2f2f2;
 }
 
@@ -419,7 +422,7 @@ list .deletable-item {
 list .deletable-item > :first-child {
   -webkit-box-align: center;
   -webkit-box-flex: 1;
-  -webkit-padding-end: 3px;
+  -webkit-padding-end: 5px;
   display: -webkit-box;
 }
 
@@ -434,7 +437,7 @@ list .close-button {
   width: 16px;
 }
 
-list > *:not(:hover) .close-button,
+list > *:not(:hover):not([editing]) .close-button,
 list[disabled] .close-button,
 list .close-button[disabled] {
   opacity: 0;
@@ -449,6 +452,25 @@ list .close-button:active {
   background-image: url("../../../app/theme/close_bar_p.png");
 }
 
+list .static-text {
+  overflow: hidden;
+  text-overflow: ellipsis;
+  white-space: nowrap;
+}
+
+list > :not([editing]) [editmode=true] {
+  display: none;
+}
+
+list > [editing] [editmode=false] {
+  display: none;
+}
+
+list > [editing] input:invalid {
+  /* TODO(stuartmorgan): Replace with validity badge */
+  background-color: pink;
+}
+
 .left-side-table {
   display: -webkit-box;
 }

chrome/browser/resources/options/search_engine_manager.css

@@ -81,22 +81,3 @@
   margin: 0;
   width: 100%;
 }
-
-#searchEngineList .static-text {
-  overflow: hidden;
-  text-overflow: ellipsis;
-  white-space: nowrap;
-}
-
-#searchEngineList > :not([editing]) [editmode=true] {
-  display: none;
-}
-
-#searchEngineList > [editing] [editmode=false] {
-  display: none;
-}
-
-#searchEngineList input.invalid {
-  /* TODO(stuartmorgan): Replace with actual badging */
-  background-color: pink;
-}

chrome/browser/resources/options/search_engine_manager_engine_list.js

@@ -123,16 +123,17 @@ cr.define('options.search_engines', function() {
         nameColEl.appendChild(faviconDivEl);
       }
 
-      var nameEl = this.createEditableTextCell_(nameText);
+      var nameEl = this.createEditableTextCell(nameText, this.isPlaceholder_);
       nameColEl.appendChild(nameEl);
 
       // Then the keyword column.
-      var keywordEl = this.createEditableTextCell_(keywordText);
+      var keywordEl = this.createEditableTextCell(keywordText,
+                                                  this.isPlaceholder_);
       keywordEl.className = 'keyword-column';
       this.contentElement.appendChild(keywordEl);
 
       // And the URL column.
-      var urlEl = this.createEditableTextCell_(urlText);
+      var urlEl = this.createEditableTextCell(urlText, this.isPlaceholder_);
       urlEl.className = 'url-column';
       this.contentElement.appendChild(urlEl);
 
@@ -166,47 +167,13 @@ cr.define('options.search_engines', function() {
       this.addEventListener('commitedit', this.onEditCommitted_.bind(this));
     },
 
-    /**
-     * Returns a div containing an <input>, as well as static text if needed.
-     * @param {string} text The text of the cell.
-     * @return {HTMLElement} The HTML element for the cell.
-     * @private
-     */
-    createEditableTextCell_: function(text) {
-      var container = this.ownerDocument.createElement('div');
-
-      if (!this.isPlaceholder_) {
-        var textEl = this.ownerDocument.createElement('div');
-        textEl.className = 'static-text';
-        textEl.textContent = text;
-        textEl.setAttribute('editmode', false);
-        container.appendChild(textEl);
-      }
-
-      var inputEl = this.ownerDocument.createElement('input');
-      inputEl.type = 'text';
-      inputEl.value = text;
-      if (!this.isPlaceholder_) {
-        inputEl.setAttribute('editmode', true);
-        inputEl.staticVersion = textEl;
-      }
-      container.appendChild(inputEl);
-
-      return container;
-    },
-
-    /** @inheritDoc */
-    get initialFocusElement() {
-      return this.nameField_;
-    },
-
     /** @inheritDoc */
     get currentInputIsValid() {
       return !this.waitingForValidation_ && this.currentlyValid_;
     },
 
     /** @inheritDoc */
-    hasBeenEdited: function(e) {
+    get hasBeenEdited() {
       var engine = this.searchEngine_;
       return this.nameField_.value != engine['name'] ||
              this.keywordField_.value != engine['keyword'] ||
@@ -221,6 +188,7 @@ cr.define('options.search_engines', function() {
     onEditStarted_: function(e) {
       var editIndex = this.searchEngine_['modelIndex'];
       chrome.send('editSearchEngine', [String(editIndex)]);
+      this.startFieldValidation_();
     },
 
     /**
@@ -230,14 +198,6 @@ cr.define('options.search_engines', function() {
      */
     onEditCommitted_: function(e) {
       chrome.send('searchEngineEditCompleted', this.getInputFieldValues_());
-      // Update the static version immediately to prevent flickering before
-      // the model update callback updates the UI.
-      var editFields = [ this.nameField_, this.keywordField_, this.urlField_ ];
-      for (var i = 0; i < editFields.length; i++) {
-        var staticLabel = editFields[i].staticVersion;
-        if (staticLabel)
-          staticLabel.textContent = editFields[i].value;
-      }
     },
 
     /**
@@ -248,14 +208,12 @@ cr.define('options.search_engines', function() {
      */
     onEditCancelled_: function() {
       chrome.send('searchEngineEditCancelled');
-      var engine = this.searchEngine_;
-      this.nameField_.value = engine['name'];
-      this.keywordField_.value = engine['keyword'];
-      this.urlField_.value = engine['url'];
 
-      var editFields = [ this.nameField_, this.keywordField_, this.urlField_ ];
-      for (var i = 0; i < editFields.length; i++) {
-        editFields[i].classList.remove('invalid');
+      if (this.isPlaceholder_) {
+        var engine = this.searchEngine_;
+        this.nameField_.value = '';
+        this.keywordField_.value = '';
+        this.urlField_.value = '';
       }
       this.currentlyValid_ = !this.isPlaceholder_;
     },
@@ -290,21 +248,27 @@ cr.define('options.search_engines', function() {
     validationComplete: function(validity) {
       this.waitingForValidation_ = false;
       // TODO(stuartmorgan): Implement the full validation UI with
-      // checkmark/exclamation mark icons and tooltips.
-      if (validity['name'])
-        this.nameField_.classList.remove('invalid');
-      else
-        this.nameField_.classList.add('invalid');
-
-      if (validity['keyword'])
-        this.keywordField_.classList.remove('invalid');
-      else
-        this.keywordField_.classList.add('invalid');
-
-      if (validity['url'])
-        this.urlField_.classList.remove('invalid');
-      else
-        this.urlField_.classList.add('invalid');
+      // checkmark/exclamation mark icons and tooltips showing the errors.
+      if (validity['name']) {
+        this.nameField_.setCustomValidity('');
+      } else {
+        this.nameField_.setCustomValidity(
+            templateData.editSearchEngineInvalidTitleToolTip);
+      }
+
+      if (validity['keyword']) {
+        this.keywordField_.setCustomValidity('');
+      } else {
+        this.keywordField_.setCustomValidity(
+            templateData.editSearchEngineInvalidKeywordToolTip);
+      }
+
+      if (validity['url']) {
+        this.urlField_.setCustomValidity('');
+      } else {
+        this.urlField_.setCustomValidity(
+            templateData.editSearchEngineInvalidURLToolTip);
+      }
 
       this.currentlyValid_ = validity['name'] && validity['keyword'] &&
           validity['url'];

chrome/browser/resources/options/startup_page_manager.html

 <div id="startupPageManagerPage" class="page hidden">
   <h1 i18n-content="startupPageManagerPage"></h1>
-  <list id="startupPagesFullList" class="settings-list"></list>
+  <list id="startupPagesFullList"
+      class="settings-list startup-page-list"></list>
   <button id="startupAddButton" i18n-content="startupAddButton"></button>
 </div>

chrome/browser/resources/options/startup_page_manager.js

@@ -5,7 +5,7 @@
 cr.define('options', function() {
   const OptionsPage = options.OptionsPage;
   const ArrayDataModel = cr.ui.ArrayDataModel;
-  const ListSelectionModel = cr.ui.ListSelectionModel;
+  const ListSingleSelectionModel = cr.ui.ListSingleSelectionModel;
 
   /**
    * Encapsulated handling of startup page management page.
@@ -30,7 +30,7 @@ cr.define('options', function() {
       var list = $('startupPagesFullList');
       options.browser_options.StartupPageList.decorate(list);
       list.autoExpands = true;
-      list.selectionModel = new ListSelectionModel;
+      list.selectionModel = new ListSingleSelectionModel;
 
       // Wire up controls.
       $('startupAddButton').onclick = function(event) {