Add Show Password Button To Settings Page

This change adds a show password button directly in chrome://settings/passwords
to reduce the number of clicks necessary to view a given password.

Bug: 670986
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: If54c92d2ee6c2e64f90a5280e59a4c59af182740
Reviewed-on: https://chromium-review.googlesource.com/575143
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491454}

chrome/browser/resources/settings/passwords_and_forms_page/compiled_resources2.gyp

@@ -52,7 +52,7 @@
       'target_name': 'password_list_item',
       'dependencies': [
         '../compiled_resources2.gyp:focus_row_behavior',
-        '<(EXTERNS_GYP):passwords_private',
+        'show_password_behavior',
       ],
       'includes': ['../../../../../third_party/closure_compiler/compile_js2.gypi'],
     },
@@ -74,6 +74,13 @@
       'target_name': 'password_edit_dialog',
       'dependencies': [
         '<(DEPTH)/third_party/polymer/v1_0/components-chromium/paper-input/compiled_resources2.gyp:paper-input-extracted',
+        'show_password_behavior',
+      ],
+      'includes': ['../../../../../third_party/closure_compiler/compile_js2.gypi'],
+    },
+    {
+      'target_name': 'show_password_behavior',
+      'dependencies': [
         '<(EXTERNS_GYP):passwords_private',
       ],
       'includes': ['../../../../../third_party/closure_compiler/compile_js2.gypi'],

chrome/browser/resources/settings/passwords_and_forms_page/password_edit_dialog.html

@@ -6,6 +6,7 @@
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-input/paper-input.html">
 <link rel="import" href="../icons.html">
 <link rel="import" href="../settings_shared_css.html">
+<link rel="import" href="show_password_behavior.html">
 
 <dom-module id="password-edit-dialog">
   <template>

chrome/browser/resources/settings/passwords_and_forms_page/password_edit_dialog.js

@@ -13,20 +13,10 @@
 Polymer({
   is: 'password-edit-dialog',
 
-  properties: {
-    /**
-     * The password that is being displayed.
-     * @type {!chrome.passwordsPrivate.PasswordUiEntry}
-     */
-    item: Object,
-
-    /** Holds the plaintext password when requested. */
-    password: String,
-  },
+  behaviors: [ShowPasswordBehavior],
 
   /** @override */
   attached: function() {
-    this.password = '';
     this.$.dialog.showModal();
   },
 
@@ -35,61 +25,6 @@ Polymer({
     this.$.dialog.close();
   },
 
-  /**
-   * Gets the password input's type. Should be 'text' when password is visible
-   * or when there's federated text otherwise 'password'.
-   * @private
-   */
-  getPasswordInputType_: function() {
-    return this.password || this.item.federationText ? 'text' : 'password';
-  },
-
-  /**
-   * Gets the title text for the show/hide icon.
-   * @param {string} password
-   * @param {string} hide The i18n text to use for 'Hide'
-   * @param {string} show The i18n text to use for 'Show'
-   * @private
-   */
-  showPasswordTitle_: function(password, hide, show) {
-    return password ? hide : show;
-  },
-
-  /**
-   * Get the right icon to display when hiding/showing a password.
-   * @return {string}
-   * @private
-   */
-  getIconClass_: function() {
-    return this.password ? 'icon-visibility-off' : 'icon-visibility';
-  },
-
-  /**
-   * Gets the text of the password. Will use the value of |password| unless it
-   * cannot be shown, in which case it will be spaces. It can also be the
-   * federated text.
-   * @private
-   */
-  getPassword_: function() {
-    if (!this.item)
-      return '';
-
-    return this.item.federationText || this.password ||
-        ' '.repeat(this.item.numCharactersInPassword);
-  },
-
-  /**
-   * Handler for tapping the show/hide button. Will fire an event to request the
-   * password for this login pair.
-   * @private
-   */
-  onShowPasswordButtonTap_: function() {
-    if (this.password)
-      this.password = '';
-    else
-      this.fire('show-password', this.item.loginPair);  // Request the password.
-  },
-
   /**
    * Handler for tapping the 'done' button. Should just dismiss the dialog.
    * @private

chrome/browser/resources/settings/passwords_and_forms_page/password_list_item.html

@@ -5,6 +5,7 @@
 <link rel="import" href="../focus_row_behavior.html">
 <link rel="import" href="../settings_shared_css.html">
 <link rel="import" href="passwords_shared_css.html">
+<link rel="import" href="show_password_behavior.html">
 
 <dom-module id="password-list-item">
   <template>
@@ -18,6 +19,15 @@
          direction: rtl;
          display: flex;
       }
+
+      #password {
+        /* Since #password is an input element this is necessary to prevent
+         * Chrome from using the operating system's font instead of the Material
+         * Design font.
+         */
+        font-family: inherit;
+        font-size: inherit;
+      }
     </style>
     <div class="list-item" focus-row-container>
       <div class="website-column no-min-width"
@@ -36,21 +46,30 @@
            id="username">[[item.loginPair.username]]</div>
       <div class="password-column">
         <template is="dom-if" if="[[!item.federationText]]">
-          <!-- Password type and disabled in order to match mock. -->
           <input id="password" aria-label=$i18n{editPasswordPasswordLabel}
-              type="password" class="password-field text-elide" disabled
-              value="[[getEmptyPassword_(item.numCharactersInPassword)]]">
+              type="[[getPasswordInputType_(item, password)]]"
+              on-tap="onReadonlyInputTap_" class="password-field" readonly
+              value="[[getPassword_(item, password)]]">
           </input>
+          <button is="paper-icon-button-light" id="showPasswordButton"
+              class$="[[getIconClass_(item, password)]]"
+              on-tap="onShowPasswordButtonTap_"
+              title="[[showPasswordTitle_(password,
+                  '$i18nPolymer{hidePassword}','$i18nPolymer{showPassword}')]]"
+              focus-row-control focus-type="showPassword">
+          </button>
         </template>
         <template is="dom-if" if="[[item.federationText]]">
-          <span class="password-field text-elide">[[item.federationText]]</span>
+          <span class="password-field text-elide" id="federated">
+            [[item.federationText]]
+          </span>
         </template>
-        <button is="paper-icon-button-light" id="passwordMenu"
-            class="icon-more-vert" on-tap="onPasswordMenuTap_"
-            title="$i18n{moreActions}" focus-row-control
-            focus-type="passwordMenu">
-        </button>
       </div>
+      <button is="paper-icon-button-light" id="passwordMenu"
+          class="icon-more-vert" on-tap="onPasswordMenuTap_"
+          title="$i18n{moreActions}" focus-row-control
+          focus-type="passwordMenu">
+      </button>
     </div>
   </template>
   <script src="password_list_item.js"></script>

chrome/browser/resources/settings/passwords_and_forms_page/password_list_item.js

@@ -10,24 +10,15 @@
 Polymer({
   is: 'password-list-item',
 
-  behaviors: [FocusRowBehavior],
-
-  properties: {
-    /**
-     * The password whose info should be displayed.
-     * @type {!chrome.passwordsPrivate.PasswordUiEntry}
-     */
-    item: Array,
-  },
+  behaviors: [FocusRowBehavior, ShowPasswordBehavior],
 
   /**
-   * Creates an empty password of specified length.
-   * @param {number} length
-   * @return {string} password
+   * Selects the password on tap if revealed.
    * @private
    */
-  getEmptyPassword_: function(length) {
-    return ' '.repeat(length);
+  onReadonlyInputTap_: function() {
+    if (this.password)
+      this.$$('#password').select();
   },
 
   /**

chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html

@@ -18,7 +18,16 @@
 
 <dom-module id="passwords-section">
   <template>
-    <style include="settings-shared passwords-shared"></style>
+    <style include="settings-shared passwords-shared">
+      #savedPasswordsHeading {
+        /* This adds enough padding so that the labels are aligned with the
+         * columns. It is necessary due to the absence of the "more actions"
+         * button in the heading.
+         */
+        -webkit-padding-end: calc(
+          var(--cr-icon-ripple-size) + var(--cr-icon-button-margin-start));
+      }
+   </style>
     <div class="settings-box first">
       <settings-toggle-button id="passwordToggle" class="start primary-toggle"
           aria-label="$i18n{passwords}" no-extension-indicator

chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.js

@@ -276,19 +276,6 @@ Polymer({
             this.setPasswordExceptionsListener_));
   },
 
-  /**
-   * Sets the password in the current password dialog if the loginPair matches.
-   * @param {!chrome.passwordsPrivate.LoginPair} loginPair
-   * @param {string} password
-   */
-  setPassword: function(loginPair, password) {
-    if (this.activePassword &&
-        this.activePassword.loginPair.urls.origin == loginPair.urls.origin &&
-        this.activePassword.loginPair.username == loginPair.username) {
-      this.$$('password-edit-dialog').password = password;
-    }
-  },
-
   /**
    * Shows the edit password dialog.
    * @param {!Event} e
@@ -385,8 +372,9 @@ Polymer({
    */
   showPassword_: function(event) {
     this.passwordManager_.getPlaintextPassword(
-        /** @type {!PasswordManager.LoginPair} */ (event.detail), item => {
-          this.setPassword(item.loginPair, item.plaintextPassword);
+        /** @type {!PasswordManager.LoginPair} */ (event.detail.item.loginPair),
+        item => {
+          event.detail.password = item.plaintextPassword;
         });
   },
 

chrome/browser/resources/settings/passwords_and_forms_page/passwords_shared_css.html

@@ -13,18 +13,18 @@
 
       .website-column {
         display: flex;
-        flex: 3;
+        flex: 1;
       }
 
       .username-column {
-        flex: 2;
+        flex: 1;
         margin: 0 8px;
       }
 
       .password-column {
         align-items: center;
         display: flex;
-        flex: 2;
+        flex: 1;
       }
 
       .password-field {

chrome/browser/resources/settings/passwords_and_forms_page/show_password_behavior.html

+<link rel="import" href="chrome://resources/html/polymer.html">
+<script src="show_password_behavior.js"></script>

chrome/browser/resources/settings/passwords_and_forms_page/show_password_behavior.js

+// Copyright 2017 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.
+
+/**
+ * This behavior bundles functionality required to show a password to the user.
+ * It is used by both <password-list-item> and <password-edit-dialog>.
+ *
+ * @polymerBehavior
+ */
+var ShowPasswordBehavior = {
+
+  properties: {
+    /**
+     * The password that is being displayed.
+     * @type {!chrome.passwordsPrivate.PasswordUiEntry}
+     */
+    item: Object,
+
+    /**
+     * Holds the plaintext password when requested.
+     * Initializing it to the empty string is necessary to indicate that the
+     * password hasn't been fetched yet.
+     */
+    password: {
+      type: String,
+      value: '',
+    },
+  },
+
+  /**
+   * Gets the password input's type. Should be 'text' when password is visible
+   * or when there's federated text otherwise 'password'.
+   * @private
+   */
+  getPasswordInputType_: function() {
+    return this.password || this.item.federationText ? 'text' : 'password';
+  },
+
+  /**
+   * Gets the title text for the show/hide icon.
+   * @param {string} password
+   * @param {string} hide The i18n text to use for 'Hide'
+   * @param {string} show The i18n text to use for 'Show'
+   * @private
+   */
+  showPasswordTitle_: function(password, hide, show) {
+    return password ? hide : show;
+  },
+
+  /**
+   * Get the right icon to display when hiding/showing a password.
+   * @return {string}
+   * @private
+   */
+  getIconClass_: function() {
+    return this.password ? 'icon-visibility-off' : 'icon-visibility';
+  },
+
+  /**
+   * Gets the text of the password. Will use the value of |password| unless it
+   * cannot be shown, in which case it will be spaces. It can also be the
+   * federated text.
+   * @private
+   */
+  getPassword_: function() {
+    if (!this.item)
+      return '';
+
+    return this.item.federationText || this.password ||
+        ' '.repeat(this.item.numCharactersInPassword);
+  },
+
+  /**
+   * Handler for tapping the show/hide button. Will fire an event to request the
+   * password for this login pair.
+   * @private
+   */
+  onShowPasswordButtonTap_: function() {
+    if (this.password)
+      this.password = '';
+    else
+      this.fire('show-password', this);  // Request the password.
+  },
+};

chrome/browser/resources/settings/settings_resources.grd

@@ -714,6 +714,12 @@
       <structure name="IDR_SETTINGS_ADDRESS_EDIT_DIALOG_JS"
                  file="passwords_and_forms_page/address_edit_dialog.js"
                  type="chrome_html" />
+      <structure name="IDR_SETTINGS_SHOW_PASSWORD_BEHAVIOR_HTML"
+                 file="passwords_and_forms_page/show_password_behavior.html"
+                 type="chrome_html" />
+      <structure name="IDR_SETTINGS_SHOW_PASSWORD_BEHAVIOR_JS"
+                 file="passwords_and_forms_page/show_password_behavior.js"
+                 type="chrome_html" />
       <structure name="IDR_SETTINGS_PASSWORD_LIST_ITEM_HTML"
                  file="passwords_and_forms_page/password_list_item.html"
                  type="chrome_html" />

chrome/test/data/webui/settings/settings_passwords_section_browsertest.js

@@ -134,6 +134,20 @@ TEST_F('SettingsPasswordSectionBrowserTest', 'uiTests', function() {
     return passwordsSection;
   }
 
+  /**
+   * Helper method used to create a password list item.
+   * @param {!chrome.passwordsPrivate.PasswordUiEntry} passwordItem
+   * @return {!Object}
+   * @private
+   */
+  function createPasswordListItem(passwordItem) {
+    var passwordListItem = document.createElement('password-list-item');
+    passwordListItem.item = passwordItem;
+    document.body.appendChild(passwordListItem);
+    Polymer.dom.flush();
+    return passwordListItem;
+  }
+
   /**
    * Helper method used to create a password editing dialog.
    * @param {!chrome.passwordsPrivate.PasswordUiEntry} passwordItem
@@ -461,7 +475,7 @@ TEST_F('SettingsPasswordSectionBrowserTest', 'uiTests', function() {
       assertTrue(passwordDialog.$.showPasswordButton.hidden);
     });
 
-    test('showSavedPassword', function() {
+    test('showSavedPasswordEditDialog', function() {
       var PASSWORD = 'bAn@n@5';
       var item = FakeDataMaker.passwordEntry('goo.gl', 'bart', PASSWORD.length);
       var passwordDialog = createPasswordDialog(item);
@@ -469,8 +483,6 @@ TEST_F('SettingsPasswordSectionBrowserTest', 'uiTests', function() {
       assertFalse(passwordDialog.$.showPasswordButton.hidden);
 
       passwordDialog.password = PASSWORD;
-      passwordDialog.showPassword = true;
-
       Polymer.dom.flush();
 
       assertEquals(PASSWORD,
@@ -481,19 +493,57 @@ TEST_F('SettingsPasswordSectionBrowserTest', 'uiTests', function() {
       assertFalse(passwordDialog.$.showPasswordButton.hidden);
     });
 
+    test('showSavedPasswordListItem', function() {
+      var PASSWORD = 'bAn@n@5';
+      var item = FakeDataMaker.passwordEntry('goo.gl', 'bart', PASSWORD.length);
+      var passwordListItem = createPasswordListItem(item);
+
+      passwordListItem.password = PASSWORD;
+      Polymer.dom.flush();
+
+      assertEquals(PASSWORD, passwordListItem.$$('#password').value);
+      // Password should be visible.
+      assertEquals('text', passwordListItem.$$('#password').type);
+
+      // Hide Password Button should be shown.
+      assertTrue(passwordListItem.$$('#showPasswordButton')
+                     .classList.contains('icon-visibility-off'));
+    });
+
     // Test will timeout if event is not received.
-    test('onShowSavedPassword', function(done) {
-      var item = FakeDataMaker.passwordEntry('goo.gl', 'bart', 1);
-      var passwordDialog = createPasswordDialog(item);
+    test('onShowSavedPasswordEditDialog', function(done) {
+      var expectedItem = FakeDataMaker.passwordEntry('goo.gl', 'bart', 1);
+      var passwordDialog = createPasswordDialog(expectedItem);
 
       passwordDialog.addEventListener('show-password', function(event) {
-        assertEquals(item.loginPair.urls.origin, event.detail.urls.origin);
-        assertEquals(item.loginPair.username, event.detail.username);
+        var actualItem = event.detail.item;
+        assertEquals(
+            expectedItem.loginPair.urls.origin,
+            actualItem.loginPair.urls.origin);
+        assertEquals(
+            expectedItem.loginPair.username, actualItem.loginPair.username);
         done();
       });
 
       MockInteractions.tap(passwordDialog.$.showPasswordButton);
     });
+
+    test('onShowSavedPasswordListItem', function(done) {
+      var expectedItem = FakeDataMaker.passwordEntry('goo.gl', 'bart', 1);
+      var passwordListItem = createPasswordListItem(expectedItem);
+
+      passwordListItem.addEventListener('show-password', function(event) {
+        var actualItem = event.detail.item;
+        assertEquals(
+            expectedItem.loginPair.urls.origin,
+            actualItem.loginPair.urls.origin);
+        assertEquals(
+            expectedItem.loginPair.username, actualItem.loginPair.username);
+        done();
+      });
+
+      MockInteractions.tap(passwordListItem.$$('#showPasswordButton'));
+    });
   });
 
   mocha.run();