Allow editing passwords in settings/passwords

chrome/browser/resources/options/password_manager_list.js

Index: chrome/browser/resources/options/password_manager_list.js
diff --git a/chrome/browser/resources/options/password_manager_list.js b/chrome/browser/resources/options/password_manager_list.js
index 2a77ea3281a64156448e79ac0329d683d30028e7..41764170c494964adf56fd3b5334c3f20312f75d 100644
--- a/chrome/browser/resources/options/password_manager_list.js
+++ b/chrome/browser/resources/options/password_manager_list.js
@@ -6,6 +6,8 @@ cr.define('options.passwordManager', function() {
   /** @const */ var ArrayDataModel = cr.ui.ArrayDataModel;
   /** @const */ var DeletableItemList = options.DeletableItemList;
   /** @const */ var DeletableItem = options.DeletableItem;
+  /** @const */ var InlineEditableItemList = options.InlineEditableItemList;
+  /** @const */ var InlineEditableItem = options.InlineEditableItem;
   /** @const */ var List = cr.ui.List;
 
   /**
@@ -16,7 +18,7 @@ cr.define('options.passwordManager', function() {
    * @param {boolean} showPasswords If true, add a button to the element to
    *     allow the user to reveal the saved password.
    * @constructor
-   * @extends {cr.ui.ListItem}
+   * @extends {options.InlineEditableItem}
    */
   function PasswordListItem(dataModel, entry, showPasswords) {
     var el = cr.doc.createElement('div');
@@ -29,110 +31,253 @@ cr.define('options.passwordManager', function() {
   }
 
   PasswordListItem.prototype = {
-    __proto__: DeletableItem.prototype,
+    __proto__: InlineEditableItem.prototype,
 
     /** @override */
     decorate: function(showPasswords) {
-      DeletableItem.prototype.decorate.call(this);
+      InlineEditableItem.prototype.decorate.call(this);
 
-      // The URL of the site.
-      var urlLabel = this.ownerDocument.createElement('div');
-      urlLabel.classList.add('favicon-cell');
-      urlLabel.classList.add('weakrtl');
-      urlLabel.classList.add('url');
-      urlLabel.setAttribute('title', this.url);
-      urlLabel.textContent = this.url;
+      this.isPlaceholder = !this.url;
 
-      // The favicon URL is prefixed with "origin/", which essentially removes
-      // the URL path past the top-level domain and ensures that a scheme (e.g.,
-      // http) is being used. This ensures that the favicon returned is the
-      // default favicon for the domain and that the URL has a scheme if none
-      // is present in the password manager.
-      urlLabel.style.backgroundImage = getFaviconImageSet(
-          'origin/' + this.url, 16);
-      this.contentElement.appendChild(urlLabel);
+      // The URL of the site.
+      this.contentElement.appendChild(this.setupUrlElement());
 
-      // The stored username.
-      var usernameLabel = this.ownerDocument.createElement('div');
-      usernameLabel.className = 'name';
-      usernameLabel.textContent = this.username;
-      usernameLabel.title = this.username;
-      this.contentElement.appendChild(usernameLabel);
+      // The username.
+      this.contentElement.appendChild(this.setupUsernameElement());
 
-      // The stored password.
+      // The password.

[Dan Beam, 2014/08/26 17:39:06]

^ i don't think this style of comments is very useful

[jaekyeom, 2014/08/27 12:28:53]

Done.

       var passwordInputDiv = this.ownerDocument.createElement('div');
       passwordInputDiv.className = 'password';
 
       // The password input field.
       var passwordInput = this.ownerDocument.createElement('input');
       passwordInput.type = 'password';
-      passwordInput.className = 'inactive-password';
-      passwordInput.readOnly = true;
-      passwordInput.value = showPasswords ? this.password : '********';
-      passwordInputDiv.appendChild(passwordInput);
+      if (!this.isPlaceholder) {
+        passwordInput.className = 'inactive-password';
+        passwordInput.readOnly = true;
+        passwordInput.value = showPasswords ? this.password : '********';
+      }
       this.passwordField = passwordInput;
 
-      // The show/hide button.
-      if (showPasswords) {
-        var button = this.ownerDocument.createElement('button');
-        button.hidden = true;
-        button.className = 'list-inline-button custom-appearance';
-        button.textContent = loadTimeData.getString('passwordShowButton');
-        button.addEventListener('click', this.onClick_.bind(this), true);
-        button.addEventListener('mousedown', function(event) {
+      // Makes the password input field editable.
+      this.addEditField(passwordInput, null);
+
+      // Keeps the password validity updated.
+      this.updatePasswordValidity();
+      var thisListItem = this;
+      passwordInput.addEventListener('input', function(event) {
+        thisListItem.updatePasswordValidity();
+      });

[Dan Beam, 2014/08/26 17:39:06]

   passwordInput.addEventListener('input', function(event) {
     this.updatePasswordValidity();
   }.bind(this));

[jaekyeom, 2014/08/27 12:28:53]

Done.

+
+      passwordInputDiv.appendChild(passwordInput);
+
+      // The list-inline buttons.
+      if (!this.isPlaceholder) {
+        // The container of the list-inline buttons.
+        var buttonsContainer = this.ownerDocument.createElement('div');
+        buttonsContainer.className = 'list-inline-buttons-container';
+
+        var mousedownEventHandler = function(event) {
           // Don't focus on this button by mousedown.
           event.preventDefault();
           // Don't handle list item selection. It causes focus change.
           event.stopPropagation();
-        }, false);
-        passwordInputDiv.appendChild(button);
-        this.passwordShowButton = button;
+        };
+
+        // The overwrite button.
+        var overwriteButton = this.ownerDocument.createElement('button');
+        overwriteButton.className =
+            'list-inline-button custom-appearance overwrite-button';
+        overwriteButton.textContent = loadTimeData.getString(
+            'passwordOverwriteButton');
+        overwriteButton.addEventListener(
+            'click', this.onClickOverwriteButton_.bind(this), true);
+        overwriteButton.addEventListener(
+            'mousedown', mousedownEventHandler, false);
+        buttonsContainer.appendChild(overwriteButton);
+        this.passwordOverwriteButton = overwriteButton;
+
+        // The show/hide button.
+        if (showPasswords) {
+          var button = this.ownerDocument.createElement('button');
+          button.className = 'list-inline-button custom-appearance';
+          button.textContent = loadTimeData.getString('passwordShowButton');
+          button.addEventListener(
+              'click', this.onClickShowButton_.bind(this), true);
+          button.addEventListener('mousedown', mousedownEventHandler, false);
+          buttonsContainer.appendChild(button);
+          this.passwordShowButton = button;
+        }
+
+        passwordInputDiv.appendChild(buttonsContainer);
       }
 
       this.contentElement.appendChild(passwordInputDiv);
+
+      // Adds the event listeners for editing.
+      this.addEventListener('canceledit', this.onEditCancelled_);

[Dan Beam, 2014/08/26 17:39:06]

s/onEditCancelled_/resetInputs/

[jaekyeom, 2014/08/27 12:28:53]

Done.

+      this.addEventListener('commitedit', this.onEditCommitted_);

[Dan Beam, 2014/08/26 17:39:06]

s/onEditCommitted_/finishEdit/

[jaekyeom, 2014/08/27 12:28:53]

Done.

+    },
+
+    /**
+     * Constructs and returns the URL element for this item.
+     * @return {HTMLElement} The URL element.

[Dan Beam, 2014/08/26 17:39:06]

@protected

[jaekyeom, 2014/08/27 12:28:53]

Done.

+     */
+    setupUrlElement: function() {
+      var urlEl = this.ownerDocument.createElement('div');
+      urlEl.classList.add('favicon-cell');
+      urlEl.classList.add('weakrtl');
+      urlEl.classList.add('url');
+      urlEl.setAttribute('title', this.url);
+      urlEl.textContent = this.url;
+
+      // The favicon URL is prefixed with "origin/", which essentially removes
+      // the URL path past the top-level domain and ensures that a scheme (e.g.,
+      // http) is being used. This ensures that the favicon returned is the
+      // default favicon for the domain and that the URL has a scheme if none is
+      // present in the password manager.
+      urlEl.style.backgroundImage = getFaviconImageSet(
+          'origin/' + this.url, 16);
+      return urlEl;
+    },
+
+    /**
+     * Constructs and returns the username element for this item.
+     * @return {HTMLElement} The username element.

[Dan Beam, 2014/08/26 17:39:06]

@protected on all the methods you @override later (or @private if they're not
overridden)

[jaekyeom, 2014/08/27 12:28:53]

Done.

+     */
+    setupUsernameElement: function() {
+      var usernameEl = this.ownerDocument.createElement('div');
+      usernameEl.className = 'name';
+      usernameEl.textContent = this.username;
+      usernameEl.title = this.username;
+      return usernameEl;
     },
 
     /** @override */
     selectionChanged: function() {
-      var input = this.passwordField;
-      var button = this.passwordShowButton;
-      // The button doesn't exist when passwords can't be shown.
-      if (!button)
+      InlineEditableItem.prototype.selectionChanged.call(this);
+
+      // Don't set 'inactive-password' class for the placeholder so that it
+      // shows the background and the borders.
+      if (this.isPlaceholder)
         return;
 
+      var input = this.passwordField;
       if (this.selected) {
         input.classList.remove('inactive-password');

[Dan Beam, 2014/08/26 17:39:06]

remove curlies

[jaekyeom, 2014/08/27 12:28:53]

Done.

-        button.hidden = false;
       } else {
         input.classList.add('inactive-password');
-        button.hidden = true;
       }
     },
 
+    /** @override */
+    get currentInputIsValid() {
+      return !!this.passwordField.value;
+    },
+
+    /**
+     * Returns if the password has been edited.
+     * @return {boolean} Whether the password has been edited.
+     */
+    passwordHasBeenEdited: function() {
+      return this.passwordField.value != this.password || this.overwriting;
+    },
+
+    /** @override */
+    get hasBeenEdited() {
+      return this.passwordHasBeenEdited();
+    },
+
     /**
-     * Reveals the plain text password of this entry.
+     * Reveals the plain text password of this entry. Never called for the Add
+     * New Entry row.
+     * @param {string} password The plain text password.
      */
     showPassword: function(password) {
-      this.passwordField.value = password;
+      this.overwriting = false;
+      this.password = password;
+      this.setPasswordFieldValue_(password);
       this.passwordField.type = 'text';
+      this.passwordField.readOnly = false;
 
+      this.passwordOverwriteButton.hidden = true;
       var button = this.passwordShowButton;
       if (button)
         button.textContent = loadTimeData.getString('passwordHideButton');
     },
 
     /**
-     * Hides the plain text password of this entry.
+     * Hides the plain text password of this entry. Never called for the Add
+     * New Entry row.
      */
     hidePassword: function() {
       this.passwordField.type = 'password';
+      this.passwordField.readOnly = true;
 
+      this.passwordOverwriteButton.hidden = false;
       var button = this.passwordShowButton;
       if (button)
         button.textContent = loadTimeData.getString('passwordShowButton');
     },
 
     /**
+     * Resets the input fields to their original values and states.
+     */
+    resetInputs: function() {
+      this.finishOverwriting_();
+      this.setPasswordFieldValue_(this.password);
+    },
+
+    /**
+     * Commits the new data to the browser.
+     */
+    finishEdit: function() {
+      this.password = this.passwordField.value;
+      this.finishOverwriting_();
+      PasswordManager.updatePassword(
+          this.getOriginalIndex_(), this.passwordField.value);
+    },
+
+    /**
+     * Called with the response of the browser, which indicates the validity of
+     * the URL.
+     * @param {string} url The URL.
+     * @param {boolean} valid The validity of the URL.
+     */
+    originValidityCheckComplete: function(url, valid) {
+      // Does nothing here.

[Dan Beam, 2014/08/26 17:39:06]

^ probably OK to remove this comment

[jaekyeom, 2014/08/27 12:28:53]

Done.

+    },
+
+    /**
+     * Updates the custom validity of the password input field.
+     */
+    updatePasswordValidity: function() {
+      this.passwordField.setCustomValidity(this.passwordField.value ? '' : ' ');
+    },
+
+    /**
+     * Finishes password overwriting.
+     * @private
+     */
+    finishOverwriting_: function() {
+      if (!this.overwriting)
+        return;
+      this.overwriting = false;
+      this.passwordOverwriteButton.hidden = false;
+      this.passwordField.readOnly = true;
+    },
+
+    /**
+     * Sets the value of the password input field.
+     * @param {string} password The new value.
+     * @private
+     */
+    setPasswordFieldValue_: function(password) {
+      this.passwordField.value = password;
+      this.updatePasswordValidity();
+    },
+
+    /**
      * Get the original index of this item in the data model.
      * @return {number} The index.
      * @private
@@ -143,11 +288,29 @@ cr.define('options.passwordManager', function() {
     },
 
     /**
+     * Called when clicking the overwrite button. Allows the user to overwrite
+     * the hidden password.
+     * @param {Event} event The click event.
+     * @private
+     */
+    onClickOverwriteButton_: function(event) {
+      this.overwriting = true;
+      this.passwordOverwriteButton.hidden = true;
+
+      this.setPasswordFieldValue_('');
+      this.passwordField.readOnly = false;
+      this.passwordField.focus();
+    },
+
+    /**
      * On-click event handler. Swaps the type of the input field from password
      * to text and back.
      * @private
      */
-    onClick_: function(event) {
+    onClickShowButton_: function(event) {
+      // Prevents committing an edit.
+      this.resetInputs();
+
       if (this.passwordField.type == 'password') {
         // After the user is authenticated, showPassword() will be called.
         PasswordManager.requestShowPassword(this.getOriginalIndex_());
@@ -157,11 +320,30 @@ cr.define('options.passwordManager', function() {
     },
 
     /**
+     * Called when cancelling an edit.
+     * @param {Event} event The canceledit event.
+     * @private
+     */
+    onEditCancelled_: function(event) {
+      this.resetInputs();
+    },
+
+    /**
+     * Called when committing an edit.
+     * @param {Event} event The commitedit event.
+     * @private
+     */
+    onEditCommitted_: function(event) {
+      this.finishEdit();
+    },
+
+    /**
      * Get and set the URL for the entry.
      * @type {string}
      */
     get url() {
-      return this.dataItem[0];
+      var url = this.dataItem[0];
+      return url ? url : '';

[Dan Beam, 2014/08/26 17:39:06]

return this.dataItem[0] || '';

[jaekyeom, 2014/08/27 12:28:53]

Done.

     },
     set url(url) {
       this.dataItem[0] = url;
@@ -172,7 +354,8 @@ cr.define('options.passwordManager', function() {
      * @type {string}
      */
     get username() {
-      return this.dataItem[1];
+      var username = this.dataItem[1];
+      return username ? username : '';

[Dan Beam, 2014/08/26 17:39:06]

return this.dataItem[1] || '';

[jaekyeom, 2014/08/27 12:28:53]

Done.

     },
     set username(username) {
       this.dataItem[1] = username;
@@ -183,7 +366,8 @@ cr.define('options.passwordManager', function() {
      * @type {string}
      */
     get password() {
-      return this.dataItem[2];
+      var password = this.dataItem[2];
+      return password ? password : '';

[Dan Beam, 2014/08/26 17:39:06]

return this.dataItem[2] || '';

[jaekyeom, 2014/08/27 12:28:53]

Done.

     },
     set password(password) {
       this.dataItem[2] = password;
@@ -191,6 +375,126 @@ cr.define('options.passwordManager', function() {
   };
 
   /**
+   * Creates a new passwords list item for the Add New Entry row.
+   * @param {ArrayDataModel} dataModel The data model that contains this item.
+   * @param {boolean} showPasswords Kept for the consistency.

[Dan Beam, 2014/08/26 17:39:06]

what does "Kept for the consistency" mean?

[jaekyeom, 2014/08/27 12:28:53]

Sorry, it was unclear.
|showPasswords| was meaningless to PasswordAddRowListItem, but it was passed to
PasswordListItem.prototype.decorate in order to make all PasswordListItems get
the same |showPasswords| value.
But that also seemed meaningless so I've removed it.

+   * @constructor
+   * @extends {options.passwordManager.PasswordListItem}
+   */
+  function PasswordAddRowListItem(dataModel, showPasswords) {
+    var el = cr.doc.createElement('div');
+    el.dataItem = [];
+    el.dataModel = dataModel;
+    el.__proto__ = PasswordAddRowListItem.prototype;
+    el.decorate(showPasswords);
+
+    return el;
+  }
+
+  PasswordAddRowListItem.prototype = {
+    __proto__: PasswordListItem.prototype,
+
+    /** @override */
+    decorate: function(showPasswords) {
+      PasswordListItem.prototype.decorate.call(this, showPasswords);
+
+      this.urlField.placeholder = loadTimeData.getString(
+          'newPasswordUrlFieldPlaceholder');
+      this.usernameField.placeholder = loadTimeData.getString(
+          'newPasswordUsernameFieldPlaceholder');
+      this.passwordField.placeholder = loadTimeData.getString(
+          'newPasswordPasswordFieldPlaceholder');
+
+      // Sets the validity of the URL initially.
+      this.setUrlValid_(false);
+    },
+
+    /** @override */
+    setupUrlElement: function() {
+      var urlEl = this.createEditableTextCell('');
+      urlEl.classList.add('favicon-cell');
+      urlEl.classList.add('weakrtl');
+      urlEl.classList.add('url');
+
+      var urlField = urlEl.querySelector('input');
+      urlField.addEventListener('input', this.onUrlInput_.bind(this));
+      this.urlField = urlField;
+
+      return urlEl;
+    },
+
+    /** @override */
+    setupUsernameElement: function() {
+      var usernameEl = this.createEditableTextCell('');
+      usernameEl.className = 'name';
+
+      this.usernameField = usernameEl.querySelector('input');
+
+      return usernameEl;
+    },
+
+    /** @override */
+    get currentInputIsValid() {
+      return this.urlValidityKnown && this.urlIsValid &&
+          this.passwordField.value;
+    },
+
+    /** @override */
+    get hasBeenEdited() {
+      return this.urlField.value || this.usernameField.value ||
+          this.passwordHasBeenEdited();
+    },
+
+    /** @override */
+    resetInputs: function() {
+      PasswordListItem.prototype.resetInputs.call(this);
+
+      this.urlField.value = '';
+      this.usernameField.value = '';
+
+      this.setUrlValid_(false);
+    },
+
+    /** @override */
+    finishEdit: function() {
+      var newUrl = this.urlField.value;
+      var newUsername = this.usernameField.value;
+      var newPassword = this.passwordField.value;
+      this.resetInputs();
+
+      PasswordManager.addPassword(newUrl, newUsername, newPassword);
+    },
+
+    /** @override */
+    originValidityCheckComplete: function(url, valid) {
+      if (url == this.urlField.value)
+        this.setUrlValid_(valid);
+    },
+
+    /**
+     * Updates whether the URL in the input is valid.
+     * @param {boolean} valid The validity of the URL.
+     * @private
+     */
+    setUrlValid_: function(valid) {
+      this.urlIsValid = valid;
+      this.urlValidityKnown = true;
+      if (this.urlField)
+        this.urlField.setCustomValidity(valid ? '' : ' ');
+    },
+
+    /**
+     * Called when inputting to a URL input.
+     * @param {Event} event The input event.
+     * @private
+     */
+    onUrlInput_: function(event) {
+      this.urlValidityKnown = false;
+      PasswordManager.checkOriginValidityForAdding(this.urlField.value);
+    },
+  };
+
+  /**
    * Creates a new PasswordExceptions list item.
    * @param {Array} entry A pair of the form [url, username].
    * @constructor
@@ -246,12 +550,12 @@ cr.define('options.passwordManager', function() {
   /**
    * Create a new passwords list.
    * @constructor
-   * @extends {cr.ui.List}
+   * @extends {options.InlineEditableItemList}
    */
   var PasswordsList = cr.ui.define('list');
 
   PasswordsList.prototype = {
-    __proto__: DeletableItemList.prototype,
+    __proto__: InlineEditableItemList.prototype,
 
     /**
      * Whether passwords can be revealed or not.
@@ -262,7 +566,7 @@ cr.define('options.passwordManager', function() {
 
     /** @override */
     decorate: function() {
-      DeletableItemList.prototype.decorate.call(this);
+      InlineEditableItemList.prototype.decorate.call(this);
       Preferences.getInstance().addEventListener(
           'profile.password_manager_allow_show_passwords',
           this.onPreferenceChanged_.bind(this));
@@ -285,7 +589,10 @@ cr.define('options.passwordManager', function() {
       if (loadTimeData.getBoolean('disableShowPasswords'))
         showPasswords = false;
 
-      return new PasswordListItem(this.dataModel, entry, showPasswords);
+      if (entry)
+        return new PasswordListItem(this.dataModel, entry, showPasswords);
+
+      return new PasswordAddRowListItem(this.dataModel, showPasswords);
     },
 
     /** @override */