Bug 1564528 - Stop showing the modal password change dialog after password generation. r=MattN
authorTim Nguyen <ntim.bugs@gmail.com>
Thu, 11 Jul 2019 06:00:57 +0000
changeset 482395 d0e7b6c6b236b0100df09dacf4b0ad4778f63789
parent 482394 ec30e63fed04266be39c995366960e4ab3504574
child 482396 87130f4270cb903ec4230c074b04da9d7878b326
push id36280
push userncsoregi@mozilla.com
push dateThu, 11 Jul 2019 23:02:03 +0000
treeherdermozilla-central@d80023d342ce [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1564528
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 1564528 - Stop showing the modal password change dialog after password generation. r=MattN Differential Revision: https://phabricator.services.mozilla.com/D37646
toolkit/components/passwordmgr/LoginManagerParent.jsm
toolkit/components/passwordmgr/test/browser/browser.ini
toolkit/components/passwordmgr/test/browser/browser_generated_password_doorhangers.js
toolkit/components/passwordmgr/test/browser/browser_private_window.js
toolkit/components/passwordmgr/test/browser/form_password_change.html
toolkit/components/passwordmgr/test/browser/subtst_privbrowsing_2.html
--- a/toolkit/components/passwordmgr/LoginManagerParent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerParent.jsm
@@ -559,41 +559,47 @@ this.LoginManagerParent = {
     });
 
     // If we didn't find a username field, but seem to be changing a
     // password, allow the user to select from a list of applicable
     // logins to update the password for.
     if (!usernameField && oldPasswordField && logins.length > 0) {
       let prompter = this._getPrompter(browser, openerTopWindowID);
 
+      let { browsingContext } = browser;
+      let framePrincipalOrigin =
+        browsingContext.currentWindowGlobal.documentPrincipal.origin;
+      let generatedPW = this._generatedPasswordsByPrincipalOrigin.get(
+        framePrincipalOrigin
+      );
+
       if (logins.length == 1) {
         let oldLogin = logins[0];
 
         if (oldLogin.password == formLogin.password) {
           recordLoginUse(oldLogin);
           log(
             "(Not prompting to save/change since we have no username and the " +
               "only saved password matches the new password)"
           );
           return;
         }
 
         formLogin.username = oldLogin.username;
         formLogin.usernameField = oldLogin.usernameField;
 
         prompter.promptToChangePassword(oldLogin, formLogin, dismissedPrompt);
-      } else {
+        return;
+      } else if (!generatedPW || generatedPW.value != newPasswordField.value) {
         // Note: It's possible that that we already have the correct u+p saved
         // but since we don't have the username, we don't know if the user is
         // changing a second account to the new password so we ask anyways.
-
         prompter.promptToChangePasswordWithUsernames(logins, formLogin);
+        return;
       }
-
-      return;
     }
 
     let existingLogin = null;
     // Look for an existing login that matches the form login.
     for (let login of logins) {
       let same;
 
       // If one login has a username but the other doesn't, ignore
--- a/toolkit/components/passwordmgr/test/browser/browser.ini
+++ b/toolkit/components/passwordmgr/test/browser/browser.ini
@@ -57,16 +57,18 @@ support-files =
 skip-if = (os == "linux") || (os == "mac") # Bug 1337606
 [browser_entry_point_telemetry.js]
 [browser_exceptions_dialog.js]
 [browser_focus_before_first_DOMContentLoaded.js]
 support-files =
   file_focus_before_DOMContentLoaded.sjs
 [browser_formless_submit_chrome.js]
 [browser_generated_password_doorhangers.js]
+support-files =
+  form_password_change.html
 [browser_hasInsecureLoginForms.js]
 skip-if = verify
 [browser_hasInsecureLoginForms_streamConverter.js]
 [browser_http_autofill.js]
 skip-if = verify
 [browser_hidden_document_autofill.js]
 skip-if = os == "win" && os_version == "10.0" && debug # bug 1530935
 [browser_insecurePasswordConsoleWarning.js]
@@ -87,9 +89,9 @@ tags = clipboard
 [browser_passwordmgr_fields.js]
 [browser_passwordmgr_observers.js]
 [browser_passwordmgr_sort.js]
 [browser_passwordmgr_switchtab.js]
 [browser_passwordmgrdlg.js]
 [browser_private_window.js]
 support-files =
   subtst_privbrowsing_1.html
-  subtst_privbrowsing_2.html
+  form_password_change.html
--- a/toolkit/components/passwordmgr/test/browser/browser_generated_password_doorhangers.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_generated_password_doorhangers.js
@@ -8,30 +8,35 @@
 
 // The origin for the test URIs.
 const TEST_ORIGIN = "https://example.com";
 const FORM_PAGE_PATH =
   "/browser/toolkit/components/passwordmgr/test/browser/form_basic.html";
 const passwordInputSelector = "#form-basic-password";
 const usernameInputSelector = "#form-basic-username";
 
-let login1;
-async function addOneLogin() {
-  login1 = LoginTestUtils.testData.formLogin({
+async function addLogin({ username, password }) {
+  const login = LoginTestUtils.testData.formLogin({
     origin: "https://example.com",
     formActionOrigin: "https://example.com",
-    username: "username",
-    password: "pass1",
+    username,
+    password,
   });
   let storageChangedPromised = TestUtils.topicObserved(
     "passwordmgr-storage-changed",
     (_, data) => data == "addLogin"
   );
-  Services.logins.addLogin(login1);
+  Services.logins.addLogin(login);
   await storageChangedPromised;
+  return login;
+}
+
+let login1;
+function addOneLogin() {
+  login1 = addLogin({ username: "username", password: "pass1" });
 }
 
 function openACPopup(popup, browser, inputSelector) {
   return new Promise(async resolve => {
     let promiseShown = BrowserTestUtils.waitForEvent(popup, "popupshown");
 
     await SimpleTest.promiseFocus(browser);
     info("content window focused");
@@ -261,8 +266,50 @@ add_task(async function autocomplete_gen
         15,
         "Doorhanger password field has generated 15-char value"
       );
       is(usernameValue, "user1", "Doorhanger username field was popuplated");
       notif.remove();
     }
   );
 });
+
+add_task(async function password_change_without_username() {
+  const CHANGE_FORM_PATH =
+    "/browser/toolkit/components/passwordmgr/test/browser/form_password_change.html";
+  await BrowserTestUtils.withNewTab(
+    {
+      gBrowser,
+      url: TEST_ORIGIN + CHANGE_FORM_PATH,
+    },
+    async function(browser) {
+      await SimpleTest.promiseFocus(browser.ownerGlobal);
+      // Save 2nd login different from the 1st one
+      addLogin({
+        username: "username2",
+        password: "pass2",
+      });
+
+      // Make the 2nd field use a generated password
+      await doFillGeneratedPasswordContextMenuItem(browser, "#newpass");
+
+      // Submit the form
+      await ContentTask.spawn(browser, null, function() {
+        content.document.querySelector("#form").submit();
+      });
+
+      // Check a non-dismissed prompt was shown
+      let notif = getCaptureDoorhanger("password-save");
+      ok(notif && !notif.dismissed, "Non-dismissed notification was created");
+
+      let { passwordValue, usernameValue } = await checkPromptContents(
+        notif.anchorElement
+      );
+      is(
+        passwordValue.length,
+        15,
+        "Doorhanger password field has generated 15-char value"
+      );
+      is(usernameValue, "", "Doorhanger username field has no value");
+      notif.remove();
+    }
+  );
+});
--- a/toolkit/components/passwordmgr/test/browser/browser_private_window.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_private_window.js
@@ -141,17 +141,17 @@ let login = new nsLoginInfo(
   "https://example.com",
   null,
   "notifyu1",
   "notifyp1",
   "user",
   "pass"
 );
 const form1Url = `https://example.com/${DIRECTORY_PATH}subtst_privbrowsing_1.html`;
-const form2Url = `https://example.com/${DIRECTORY_PATH}subtst_privbrowsing_2.html`;
+const form2Url = `https://example.com/${DIRECTORY_PATH}form_password_change.html`;
 const authUrl = `https://example.com/${DIRECTORY_PATH}authenticate.sjs`;
 
 let normalWin;
 let privateWin;
 
 // XXX: Note that tasks are currently run in sequence. Some tests may assume the state
 // resulting from successful or unsuccessful logins in previous tasks
 
rename from toolkit/components/passwordmgr/test/browser/subtst_privbrowsing_2.html
rename to toolkit/components/passwordmgr/test/browser/form_password_change.html