Bug 1572478 - Don't prompt for master password when re-rendering the login-item. r=MattN
authorJared Wein <jwein@mozilla.com>
Thu, 29 Aug 2019 15:44:24 +0000
changeset 551164 d01e6a3eea5ca1c9e91ca4440a2edc35804f481f
parent 551163 8f4bcd61ad953f6a08447ef40f08b037c22e5140
child 551165 0bd14a48e5b8ba743292f475cc597d27783d30d5
push id11865
push userbtara@mozilla.com
push dateMon, 02 Sep 2019 08:54:37 +0000
treeherdermozilla-beta@37f59c4671b3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1572478
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1572478 - Don't prompt for master password when re-rendering the login-item. r=MattN We should only prompt for master password when copying a password, or when changing from a hidden to revealed password. This patch maintains the revealed state when going from readonly to edit mode of the login. When leaving edit mode, whether through cancel or save, the revealed state is reset. Differential Revision: https://phabricator.services.mozilla.com/D43883
browser/components/aboutlogins/content/components/login-item.js
browser/components/aboutlogins/tests/browser/browser_updateLogin.js
browser/components/aboutlogins/tests/chrome/test_login_item.html
--- a/browser/components/aboutlogins/content/components/login-item.js
+++ b/browser/components/aboutlogins/content/components/login-item.js
@@ -138,17 +138,17 @@ export default class LoginItem extends H
     }
     this._copyUsernameButton.disabled = !this._login.username;
     document.l10n.setAttributes(
       this._saveChangesButton,
       this.dataset.isNewLogin
         ? "login-item-save-new-button"
         : "login-item-save-changes-button"
     );
-    await this._updatePasswordRevealState();
+    this._updatePasswordRevealState();
   }
 
   updateBreaches(breachesByLoginGUID) {
     this._breachesMap = breachesByLoginGUID;
     this.render();
   }
 
   dismissBreachAlert() {
@@ -193,17 +193,26 @@ export default class LoginItem extends H
         if (!originValue.match(/:\/\//)) {
           this._originInput.value = "https://" + originValue;
         }
         break;
       }
       case "click": {
         let classList = event.currentTarget.classList;
         if (classList.contains("reveal-password-checkbox")) {
-          await this._updatePasswordRevealState();
+          if (this._revealCheckbox.checked) {
+            let masterPasswordAuth = await new Promise(resolve => {
+              window.AboutLoginsUtils.promptForMasterPassword(resolve);
+            });
+            if (!masterPasswordAuth) {
+              this._revealCheckbox.checked = false;
+              return;
+            }
+          }
+          this._updatePasswordRevealState();
 
           let method = this._revealCheckbox.checked ? "show" : "hide";
           recordTelemetryEvent({ object: "password", method });
           return;
         }
 
         if (classList.contains("cancel-button")) {
           let wasExistingLogin = !!this._login.guid;
@@ -456,17 +465,17 @@ export default class LoginItem extends H
   loginAdded(login) {
     if (
       this._login.guid ||
       !window.AboutLoginsUtils.doLoginsMatch(login, this._loginFromForm())
     ) {
       return;
     }
 
-    this._login = login;
+    this.setLogin(login);
     this.dispatchEvent(
       new CustomEvent("AboutLoginsLoginSelected", {
         bubbles: true,
         composed: true,
         detail: login,
       })
     );
   }
@@ -478,19 +487,18 @@ export default class LoginItem extends H
    * @param {login} login The login that was modified in storage. The login object is
    *                      a plain JS object representation of nsILoginInfo/nsILoginMetaInfo.
    */
   loginModified(login) {
     if (this._login.guid != login.guid) {
       return;
     }
 
-    this._login = login;
+    this.setLogin(login);
     this._toggleEditing(false);
-    this.render();
   }
 
   /**
    * Clears the displayed login if the argument matches the currently
    * displayed login.
    *
    * @param {login} login The login that was removed from storage. The login object is
    *                      a plain JS object representation of nsILoginInfo/nsILoginMetaInfo.
@@ -581,30 +589,22 @@ export default class LoginItem extends H
     this._usernameInput.tabIndex = inputTabIndex;
     this._passwordInput.readOnly = !shouldEdit;
     this._passwordInput.tabIndex = inputTabIndex;
     if (shouldEdit) {
       this.dataset.editing = true;
       this._originInput.focus();
     } else {
       delete this.dataset.editing;
+      // Only reset the reveal checkbox when exiting 'edit' mode
+      this._revealCheckbox.checked = false;
     }
   }
 
-  async _updatePasswordRevealState() {
-    if (this._revealCheckbox.checked) {
-      let masterPasswordAuth = await new Promise(resolve => {
-        window.AboutLoginsUtils.promptForMasterPassword(resolve);
-      });
-      if (!masterPasswordAuth) {
-        this._revealCheckbox.checked = false;
-        return;
-      }
-    }
-
+  _updatePasswordRevealState() {
     let titleId = this._revealCheckbox.checked
       ? "login-item-password-reveal-checkbox-hide"
       : "login-item-password-reveal-checkbox-show";
     document.l10n.setAttributes(this._revealCheckbox, titleId);
 
     let { checked } = this._revealCheckbox;
     let inputType = checked ? "text" : "password";
     this._passwordInput.setAttribute("type", inputType);
--- a/browser/components/aboutlogins/tests/browser/browser_updateLogin.js
+++ b/browser/components/aboutlogins/tests/browser/browser_updateLogin.js
@@ -115,16 +115,25 @@ add_task(async function test_login_item(
       await test_discard_dialog(loginList._createLoginButton);
 
       let cancelButton = loginItem.shadowRoot.querySelector(".cancel-button");
       await test_discard_dialog(cancelButton);
 
       editButton.click();
       await Promise.resolve();
 
+      let revealCheckbox = loginItem.shadowRoot.querySelector(
+        ".reveal-password-checkbox"
+      );
+      revealCheckbox.click();
+      ok(
+        revealCheckbox.checked,
+        "reveal-checkbox should be checked after clicking"
+      );
+
       usernameInput.value += "-saveme";
       passwordInput.value += "-saveme";
 
       ok(loginItem.dataset.editing, "LoginItem should be in 'edit' mode");
 
       let saveChangesButton = loginItem.shadowRoot.querySelector(
         ".save-changes-button"
       );
@@ -136,16 +145,20 @@ add_task(async function test_login_item(
         return (
           updatedLogin &&
           updatedLogin.username == usernameInput.value &&
           updatedLogin.password == passwordInput.value
         );
       }, "Waiting for corresponding login in login list to update");
 
       ok(
+        !revealCheckbox.checked,
+        "reveal-checkbox should be unchecked after saving changes"
+      );
+      ok(
         !loginItem.dataset.editing,
         "LoginItem should not be in 'edit' mode after saving"
       );
       is(
         passwordInput.style.width,
         passwordInput.value.length + "ch",
         "Password field width should be correctly updated"
       );
--- a/browser/components/aboutlogins/tests/chrome/test_login_item.html
+++ b/browser/components/aboutlogins/tests/chrome/test_login_item.html
@@ -212,16 +212,23 @@ add_task(async function test_reveal_pass
   ok(!revealCheckbox.checked, "reveal-checkbox should not be checked by default");
   is(passwordInput.type, "password", "Password should be masked by default");
   revealCheckbox.click();
   ok(revealCheckbox.checked, "reveal-checkbox should be checked after clicking");
   await SimpleTest.promiseWaitForCondition(() => passwordInput.type == "text",
     "waiting for password input type to change after checking for master password");
   is(passwordInput.type, "text", "Password should be unmasked when checkbox is clicked");
 
+  let editButton = gLoginItem.shadowRoot.querySelector(".edit-button");
+  editButton.click();
+  ok(revealCheckbox.checked, "reveal-checkbox should remain checked when entering 'edit' mode");
+  gLoginItem.shadowRoot.querySelector(".cancel-button").click();
+  ok(!revealCheckbox.checked, "reveal-checkbox should be unchecked after canceling 'edit' mode");
+  revealCheckbox.click();
+
   gLoginItem.setLogin(TEST_LOGIN_2);
   ok(!revealCheckbox.checked, "reveal-checkbox should be unchecked when changing logins");
   is(passwordInput.type, "password", "Password should be masked by default when switching logins");
 });
 
 add_task(async function test_set_login_empty() {
   gLoginItem.setLogin({});
   await asyncElementRendered();