[tabbed options] more work on content settings exceptions lists

chrome/browser/resources/options/content_settings_exceptions_area.js

Index: chrome/browser/resources/options/content_settings_exceptions_area.js
diff --git a/chrome/browser/resources/options/content_settings_exceptions_area.js b/chrome/browser/resources/options/content_settings_exceptions_area.js
index f727476bee28a9e8e6e33398bf672301979f3b5a..c6e70e9c84baac336927cc085d06e80c2535e536 100644
--- a/chrome/browser/resources/options/content_settings_exceptions_area.js
+++ b/chrome/browser/resources/options/content_settings_exceptions_area.js
@@ -39,21 +39,35 @@ cr.define('options.contentSettings', function() {
     decorate: function() {
       ListItem.prototype.decorate.call(this);
 
-      // Labels for display mode.
-      var patternLabel = cr.doc.createElement('span');
-      patternLabel.textContent = this.pattern;
-      this.appendChild(patternLabel);
-
-      var settingLabel = cr.doc.createElement('span');
-      settingLabel.textContent = this.settingForDisplay();
-      settingLabel.className = 'exceptionSetting';
-      this.appendChild(settingLabel);
+      // Labels for display mode. |pattern| will be null for the 'add new
+      // exception' row.
+      if (this.pattern) {
+        this.isAddNewRow = false;
+
+        var patternLabel = cr.doc.createElement('span');
+        patternLabel.textContent = this.pattern;
+        patternLabel.className = 'exceptionPattern';
+        this.appendChild(patternLabel);
+        this.patternLabel = patternLabel;
+
+        var settingLabel = cr.doc.createElement('span');
+        settingLabel.textContent = this.settingForDisplay();
+        settingLabel.className = 'exceptionSetting';
+        this.appendChild(settingLabel);
+        this.settingLabel = settingLabel;
+      } else {
+        this.isAddNewRow = true;
+      }
 
       // Elements for edit mode.
       var input = cr.doc.createElement('input');
       input.type = 'text';
       this.appendChild(input);
-      input.className = 'exceptionInput hidden';
+      input.className = 'exceptionPattern';
+      if (this.isAddNewRow)
+        input.placeholder = templateData.addNewExceptionInstructions;
+      else
+        input.classList.add('hidden');
 
       var select = cr.doc.createElement('select');
       var optionAllow = cr.doc.createElement('option');
@@ -79,7 +93,9 @@ cr.define('options.contentSettings', function() {
       select.appendChild(optionBlock);
 
       this.appendChild(select);
-      select.className = 'exceptionSetting hidden';
+      select.className = 'exceptionSetting';
+      if (!this.isAddNewRow)
+        select.classList.add('hidden');
 
       // Used to track whether the URL pattern in the input is valid.
       // This will be true if the browser process has informed us that the
@@ -93,26 +109,23 @@ cr.define('options.contentSettings', function() {
       // empty input.
       this.inputIsValid = true;
 
-      this.patternLabel = patternLabel;
-      this.settingLabel = settingLabel;
       this.input = input;
       this.select = select;
       this.optionAllow = optionAllow;
       this.optionBlock = optionBlock;
 
       this.updateEditables();
-      if (!this.pattern)
-        input.value = templateData.examplePattern;
 
       var listItem = this;
-      this.ondblclick = function(event) {
+
+      this.addEventListener('selectedChange', function(event) {
         // Editing notifications and geolocation is disabled for now.
         if (listItem.contentType == 'notifications' ||
             listItem.contentType == 'location')
           return;
 
-        listItem.editing = true;
-      };
+        listItem.editing = listItem.selected;
+      });
 
       // Handle events on the editable nodes.
       input.oninput = function(event) {
@@ -132,32 +145,14 @@ cr.define('options.contentSettings', function() {
           case 'U+001B':  // Esc
             // Reset the inputs.
             listItem.updateEditables();
-            if (listItem.pattern)
-              listItem.maybeSetPatternValid(listItem.pattern, true);
+            listItem.setPatternValid(true);
           case 'Enter':
-            if (listItem.parentNode)
-              listItem.parentNode.focus();
+            listItem.ownerDocument.activeElement.blur();
         }
       }
 
-      function handleBlur(e) {
-        // When the blur event happens we do not know who is getting focus so we
-        // delay this a bit since we want to know if the other input got focus
-        // before deciding if we should exit edit mode.
-        var doc = e.target.ownerDocument;
-        window.setTimeout(function() {
-          var activeElement = doc.activeElement;
-          if (!listItem.contains(activeElement)) {
-            listItem.editing = false;
-          }
-        }, 50);
-      }
-
       input.addEventListener('keydown', handleKeydown);
-      input.addEventListener('blur', handleBlur);
-
       select.addEventListener('keydown', handleKeydown);
-      select.addEventListener('blur', handleBlur);
     },
 
     /**
@@ -199,19 +194,11 @@ cr.define('options.contentSettings', function() {
     },
 
     /**
-     * Update this list item to reflect whether the input is a valid pattern
-     * if |pattern| matches the text currently in the input.
-     * @param {string} pattern The pattern.
+     * Update this list item to reflect whether the input is a valid pattern.
      * @param {boolean} valid Whether said pattern is valid in the context of
      *     a content exception setting.
      */
-    maybeSetPatternValid: function(pattern, valid) {
-      // Don't do anything for messages where we are not the intended recipient,
-      // or if the response is stale (i.e. the input value has changed since we
-      // sent the request to analyze it).
-      if (pattern != this.input.value)
-        return;
-
+    setPatternValid: function(valid) {
       if (valid)
         this.input.setCustomValidity('');
       else
@@ -224,7 +211,10 @@ cr.define('options.contentSettings', function() {
      * Copy the data model values to the editable nodes.
      */
     updateEditables: function() {
-      this.input.value = this.pattern;
+      if (this.isAddNewRow)

[stuartmorgan, 2010/12/13 17:49:51]

It seems like there's a bunch of this kind of logic; what about doing a bit of
refactoring of methods and then making the new row a subclass that can override
a few places?

[Evan Stade, 2010/12/13 20:00:19]

ok, i'll do this

+        this.input.value = '';
+      else
+        this.input.value = this.pattern;
 
       if (this.setting == 'allow')
         this.optionAllow.selected = true;
@@ -241,11 +231,11 @@ cr.define('options.contentSettings', function() {
      * @type {boolean}
      */
     get editing() {
-      return this.hasAttribute('editing');
+      return this.isAddNewRow || this.hasAttribute('editing');
     },
     set editing(editing) {
       var oldEditing = this.editing;
-      if (oldEditing == editing)
+      if (!this.isAddNewRow && (oldEditing == editing))
         return;
 
       var listItem = this;
@@ -260,39 +250,37 @@ cr.define('options.contentSettings', function() {
       var optionSession = this.optionSession;
       var optionAsk = this.optionAsk;
 
-      // Just delete this row if it was added via the Add button.
-      if (!editing && !pattern && !input.value) {
-        var model = listItem.parentNode.dataModel;
-        model.splice(model.indexOf(listItem.dataItem), 1);
-        return;
-      }
-
-      // Check that we have a valid pattern and if not we do not change the
-      // editing mode.
-      if (!editing && (!this.inputValidityKnown || !this.inputIsValid)) {
-        input.focus();
-        input.select();
-        return;
+      if (!this.isAddNewRow) {
+        patternLabel.classList.toggle('hidden');
+        settingLabel.classList.toggle('hidden');
+        input.classList.toggle('hidden');
+        select.classList.toggle('hidden');
       }
 
-      patternLabel.classList.toggle('hidden');
-      settingLabel.classList.toggle('hidden');
-      input.classList.toggle('hidden');
-      select.classList.toggle('hidden');
-
-      var doc = this.ownerDocument;
-      var area = doc.querySelector('div[contentType=' +
-          listItem.contentType + '][mode=' + listItem.mode + ']');
-      area.enableAddAndEditButtons(!editing);
-
       if (editing) {
         this.setAttribute('editing', '');
         cr.ui.limitInputWidth(input, this, 20);
-        input.focus();
-        input.select();
+        // When this is called in response to the selectedChange event,
+        // the list grabs focus immediately afterwards. Thus we must delay
+        // our focus grab.
+        window.setTimeout(function() {
+          input.focus();
+          input.select();
+        }, 50);
+
+        // TODO(estade): should we insert example text here for the AddNewRow
+        // input?
       } else {
         this.removeAttribute('editing');
 
+        // Check that we have a valid pattern and if not we do not, abort
+        // changes to the exception.
+        if (!this.inputValidityKnown || !this.inputIsValid) {
+          this.updateEditables();
+          this.setPatternValid(true);
+          return;
+        }
+
         var newPattern = input.value;
 
         var newSetting;
@@ -306,20 +294,25 @@ cr.define('options.contentSettings', function() {
           newSetting = 'ask';
 
         // Empty edit - do nothing.
-        if (pattern == newPattern && newSetting == this.setting)
+        if ((pattern == newPattern && newSetting == this.setting) ||
+            (this.isAddNewRow && newPattern == '')) {
           return;
+        }
 
-        this.pattern = patternLabel.textContent = newPattern;
-        this.setting = newSetting;
-        settingLabel.textContent = this.settingForDisplay();
+        if (!this.isAddNewRow) {
+          patternLabel.textContent = newPattern;
+          settingLabel.textContent = this.settingForDisplay();
+          this.pattern = newPattern;
+          this.setting = newSetting;
+        }
 
-        if (pattern != this.pattern) {
+        if (pattern != newPattern) {
           chrome.send('removeExceptions',
                       [this.contentType, this.mode, pattern]);
         }
 
         chrome.send('setException',
-                    [this.contentType, this.mode, this.pattern, this.setting]);
+                    [this.contentType, this.mode, newPattern, newSetting]);
       }
     }
   };
@@ -342,9 +335,29 @@ cr.define('options.contentSettings', function() {
 
       this.dataModel = new ArrayDataModel([]);
 
+      this.contentType = this.parentNode.getAttribute('contentType');
+      this.mode = this.getAttribute('mode');
+
+      var exceptionList = this;
+      function handleBlur(e) {
+        // When the blur event happens we do not know who is getting focus so we
+        // delay this a bit since we want to know if the other input got focus
+        // before deciding if we should clear the selection.
+        var doc = e.target.ownerDocument;
+        window.setTimeout(function() {
+          var activeElement = doc.activeElement;
+          if (!exceptionList.contains(activeElement))
+            exceptionList.selectionModel.clear();
+        }, 50);
+      }
+
+      this.addEventListener('blur', handleBlur, true);
+
       // Whether the exceptions in this list allow an 'Ask every time' option.
       this.enableAskOption = (this.contentType == 'plugins' &&
                               templateData.enable_click_to_play);
+
+      this.reset();
     },
 
     /**
@@ -364,16 +377,6 @@ cr.define('options.contentSettings', function() {
      */
     addException: function(entry) {
       this.dataModel.push(entry);
-
-      // When an empty row is added, put it into editing mode.
-      if (!entry['displayPattern'] && !entry['setting']) {
-        var index = this.dataModel.length - 1;
-        var sm = this.selectionModel;
-        sm.anchorIndex = sm.leadIndex = sm.selectedIndex = index;
-        this.scrollIndexIntoView(index);
-        var li = this.getListItemByIndex(index);
-        li.editing = true;
-      }
     },
 
     /**
@@ -384,18 +387,23 @@ cr.define('options.contentSettings', function() {
      *     a content exception setting.
      */
     patternValidityCheckComplete: function(pattern, valid) {
-      for (var i = 0; i < this.dataModel.length; i++) {
+      for (var i = this.firstIndex_; i < this.lastIndex_; i++) {
         var listItem = this.getListItemByIndex(i);
-        if (listItem)
-          listItem.maybeSetPatternValid(pattern, valid);
+        // Don't do anything for messages for the item if it is not the intended
+        // recipient, or if the response is stale (i.e. the input value has
+        // changed since we sent the request to analyze it).
+        if (pattern == listItem.input.value)
+          listItem.setPatternValid(valid);
       }
     },
 
     /**
      * Removes all exceptions from the js model.
      */
-    clear: function() {
+    reset: function() {
       this.dataModel = new ArrayDataModel([]);
+      var addItemEntry = [];
+      this.dataModel.push(addItemEntry);
     },
 
     /**