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..723aeed0a12ab4e1039846ebfa5e9bc9a04128f6 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);
+      this.contentElement.appendChild(this.setupUrlElement());
+      this.contentElement.appendChild(this.setupUsernameElement());
 
-      // 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 stored password.
       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_();
+      passwordInput.addEventListener('input', function(event) {
+        this.updatePasswordValidity_();
+      }.bind(this));
+
+      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.resetInputs);
+      this.addEventListener('commitedit', this.finishEdit);
+    },
+
+    /**
+     * Constructs and returns the URL element for this item.
+     * @return {HTMLElement} The URL element.
+     * @protected
+     */
+    setupUrlElement: function() {
+      var urlEl = this.ownerDocument.createElement('div');
+      urlEl.classList.add('favicon-cell');
+      urlEl.classList.add('weakrtl');
+      urlEl.classList.add('url');

[Dan Beam, 2014/09/11 05:17:57]

urlEl.className += ' all the classes'

[jaekyeom, 2014/09/12 10:35:57]

Done.

+      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.
+     * @protected
+     */
+    setupUsernameElement: function() {

[Dan Beam, 2014/09/11 05:17:57]

nit: why not createUsernameElement?

[jaekyeom, 2014/09/12 10:35:57]

Done.

+      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;
 
-      if (this.selected) {
+      var input = this.passwordField;
+      if (this.selected)
         input.classList.remove('inactive-password');
-        button.hidden = false;
-      } else {
+      else
         input.classList.add('inactive-password');

[Dan Beam, 2014/09/11 05:17:57]

nit: input.classList.toggle('inactive-password', !this.selected);

[jaekyeom, 2014/09/12 10:35:57]

Done.

-        button.hidden = true;
-      }
+    },
+
+    /** @override */
+    get currentInputIsValid() {
+      return !!this.passwordField.value;
     },
 
     /**
-     * Reveals the plain text password of this entry.
+     * Returns if the password has been edited.
+     * @return {boolean} Whether the password has been edited.
+     * @protected
+     */
+    passwordHasBeenEdited: function() {
+      return this.passwordField.value != this.password || this.overwriting;
+    },
+
+    /** @override */
+    get hasBeenEdited() {
+      return this.passwordHasBeenEdited();
+    },
+
+    /**
+     * 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.
+     * @private
      */
-    hidePassword: function() {
+    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.
+     * @protected
+     */
+    resetInputs: function() {
+      this.finishOverwriting_();
+      this.setPasswordFieldValue_(this.password);
+    },
+
+    /**
+     * Commits the new data to the browser.
+     * @protected
+     */
+    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.

[Dan Beam, 2014/09/11 05:17:57]

can this be @protected?

[jaekyeom, 2014/09/12 10:35:57]

No, it can't. It is called by PasswordManager.

+     */
+    originValidityCheckComplete: function(url, valid) {
+    },
+
+    /**
+     * Updates the custom validity of the password input field.
+     * @private
+     */
+    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,16 +288,34 @@ 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_());
       } else {
-        this.hidePassword();
+        this.hidePassword_();
       }
     },
 
@@ -161,7 +324,7 @@ cr.define('options.passwordManager', function() {
      * @type {string}
      */
     get url() {
-      return this.dataItem[0];
+      return this.dataItem[0] || '';
     },
     set url(url) {
       this.dataItem[0] = url;
@@ -172,7 +335,7 @@ cr.define('options.passwordManager', function() {
      * @type {string}
      */
     get username() {
-      return this.dataItem[1];
+      return this.dataItem[1] || '';
     },
     set username(username) {
       this.dataItem[1] = username;
@@ -183,7 +346,7 @@ cr.define('options.passwordManager', function() {
      * @type {string}
      */
     get password() {
-      return this.dataItem[2];
+      return this.dataItem[2] || '';
     },
     set password(password) {
       this.dataItem[2] = password;
@@ -191,6 +354,125 @@ 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.
+   * @constructor
+   * @extends {options.passwordManager.PasswordListItem}
+   */
+  function PasswordAddRowListItem(dataModel) {
+    var el = cr.doc.createElement('div');
+    el.dataItem = [];
+    el.dataModel = dataModel;
+    el.__proto__ = PasswordAddRowListItem.prototype;
+    el.decorate();
+
+    return el;
+  }
+
+  PasswordAddRowListItem.prototype = {
+    __proto__: PasswordListItem.prototype,
+
+    /** @override */
+    decorate: function() {
+      PasswordListItem.prototype.decorate.call(this, false);
+
+      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);

[Dan Beam, 2014/09/11 05:17:57]

it'd arguably be better for this list element not to know about the password
manager, but i don't care that much

[jaekyeom, 2014/09/12 10:35:57]

Actually, I did this way since the original codes had used the password manager
as the interface to the browser. e.g. Call requestShowPassword -> get the result
via showPassword

+    },
+
+    /** @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 ? '' : ' ');

[Dan Beam, 2014/09/11 05:17:57]

what does the ' ' do?

[jaekyeom, 2014/09/12 10:35:57]

Done.
I added the texts.

+    },
+
+    /**
+     * 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 +528,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 +544,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));
@@ -280,6 +562,9 @@ cr.define('options.passwordManager', function() {
 
     /** @override */
     createItem: function(entry) {
+      if (!entry)

[Dan Beam, 2014/09/11 05:17:57]

why not just create the item directly instead of adding this kind of funky
code...?

[jaekyeom, 2014/09/12 10:35:57]

What does "create the item directly" mean? Using just appendChild..?
I believe it won't work correctly unless |dataModel| has an entry corresponding
to the placeholder item.

[Dan Beam, 2014/09/12 23:32:24]

ah, i see

+        return new PasswordAddRowListItem(this.dataModel);
+
       var showPasswords = this.showPasswords_;
 
       if (loadTimeData.getBoolean('disableShowPasswords'))
@@ -336,6 +621,7 @@ cr.define('options.passwordManager', function() {
 
   return {
     PasswordListItem: PasswordListItem,
+    PasswordAddRowListItem: PasswordAddRowListItem,
     PasswordExceptionsListItem: PasswordExceptionsListItem,
     PasswordsList: PasswordsList,
     PasswordExceptionsList: PasswordExceptionsList,