Bug 1571465 - Stop treating password fields we're about to fill as generated password ones. r=sfoster
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Mon, 12 Aug 2019 23:24:54 +0000
changeset 488107 21fccebed05de6fcbb24d34d817b3dbba0c10ac8
parent 488106 2758d131787315018cf4092f6dd0bd5952e47996
child 488108 a6f4fcb2f96000cf3d93d1316652670600852afc
push id113900
push usercbrindusan@mozilla.com
push dateThu, 15 Aug 2019 09:53:50 +0000
treeherdermozilla-inbound@0db07ff50ab5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfoster
bugs1571465
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 1571465 - Stop treating password fields we're about to fill as generated password ones. r=sfoster Differential Revision: https://phabricator.services.mozilla.com/D41147
toolkit/components/passwordmgr/LoginManager.jsm
toolkit/components/passwordmgr/LoginManagerContent.jsm
toolkit/components/passwordmgr/test/browser/browser_context_menu_generated_password.js
toolkit/components/passwordmgr/test/browser/browser_context_menu_iframe.js
--- a/toolkit/components/passwordmgr/LoginManager.jsm
+++ b/toolkit/components/passwordmgr/LoginManager.jsm
@@ -28,17 +28,17 @@ ChromeUtils.defineModuleGetter(
 );
 ChromeUtils.defineModuleGetter(
   this,
   "InsecurePasswordUtils",
   "resource://gre/modules/InsecurePasswordUtils.jsm"
 );
 
 XPCOMUtils.defineLazyGetter(this, "log", () => {
-  let logger = LoginHelper.createLogger("nsLoginManager");
+  let logger = LoginHelper.createLogger("LoginManager");
   return logger;
 });
 
 const MS_PER_DAY = 24 * 60 * 60 * 1000;
 
 if (Services.appinfo.processType !== Services.appinfo.PROCESS_TYPE_DEFAULT) {
   throw new Error("LoginManager.jsm should only run in the parent process");
 }
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -1001,19 +1001,19 @@ this.LoginManagerContent = {
 
     if (!LoginFormFactory.createFromField(acInputField)) {
       return;
     }
 
     if (LoginHelper.isUsernameFieldType(acInputField)) {
       this.onUsernameAutocompleted(acInputField, loginGUID);
     } else if (acInputField.hasBeenTypePassword) {
-      // Ensure the field gets re-masked in case a generated password was
-      // filled into it previously.
-      this._disableAutomaticPasswordFieldUnmasking(acInputField);
+      // Ensure the field gets re-masked and edits don't overwrite the generated
+      // password in case a generated password was filled into it previously.
+      this._stopTreatingAsGeneratedPasswordField(acInputField);
       this._highlightFilledField(acInputField);
     }
   },
 
   /**
    * A username field was filled or tabbed away from so try fill in the
    * associated password in the password field.
    */
@@ -1530,18 +1530,22 @@ this.LoginManagerContent = {
 
   _maybeStopTreatingAsGeneratedPasswordField(event) {
     let passwordField = event.target;
 
     // If the field isn't empty then keep treating it as a generated password field.
     if (passwordField.value) {
       return;
     }
+    this._stopTreatingAsGeneratedPasswordField(passwordField);
+  },
 
-    log("_maybeStopTreatingAsGeneratedPasswordField: Stopping");
+  _stopTreatingAsGeneratedPasswordField(passwordField) {
+    log("_stopTreatingAsGeneratedPasswordField");
+
     // Remove all the event listeners added in _generatedPasswordFilledOrEdited
     for (let eventType of ["blur", "change", "focus", "input"]) {
       passwordField.removeEventListener(eventType, observer, {
         capture: true,
         mozSystemGroup: true,
       });
     }
 
@@ -1629,28 +1633,16 @@ this.LoginManagerContent = {
     }
 
     if (editor.autoMaskingEnabled) {
       return;
     }
     editor.mask();
   },
 
-  _disableAutomaticPasswordFieldUnmasking(passwordField) {
-    passwordField.removeEventListener("blur", observer, {
-      capture: true,
-      mozSystemGroup: true,
-    });
-    passwordField.removeEventListener("focus", observer, {
-      capture: true,
-      mozSystemGroup: true,
-    });
-    this._togglePasswordFieldMasking(passwordField, false);
-  },
-
   /** Remove login field highlight when its value is cleared or overwritten.
    */
   _removeFillFieldHighlight(event) {
     let winUtils = event.target.ownerGlobal.windowUtils;
     winUtils.removeManuallyManagedState(event.target, AUTOFILL_STATE);
   },
 
   /**
@@ -2008,17 +2000,17 @@ this.LoginManagerContent = {
           this._highlightFilledField(usernameField);
         }
       }
 
       let doc = form.ownerDocument;
       if (passwordField.value != selectedLogin.password) {
         // Ensure the field gets re-masked in case a generated password was
         // filled into it previously.
-        this._disableAutomaticPasswordFieldUnmasking(passwordField);
+        this._stopTreatingAsGeneratedPasswordField(passwordField);
         passwordField.setUserInput(selectedLogin.password);
         let autoFilledLogin = {
           guid: selectedLogin.QueryInterface(Ci.nsILoginMetaInfo).guid,
           username: selectedLogin.username,
           usernameField: usernameField
             ? Cu.getWeakReference(usernameField)
             : null,
           password: selectedLogin.password,
--- a/toolkit/components/passwordmgr/test/browser/browser_context_menu_generated_password.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_context_menu_generated_password.js
@@ -273,19 +273,81 @@ add_task(async function fill_generated_p
           isnot(
             content.getComputedStyle(input).filter,
             "none",
             "Password field should be highlighted"
           );
           LTU.loginField.checkPasswordMasked(input, false, "after fill");
         }
       );
+
+      await openPasswordContextMenu(browser, passwordInputSelector);
+
+      // Execute the command of the first login menuitem found at the context menu.
+      let passwordChangedPromise = ContentTask.spawn(
+        browser,
+        null,
+        async function() {
+          let passwordInput = content.document.getElementById(
+            "form-basic-password"
+          );
+          await ContentTaskUtils.waitForEvent(passwordInput, "input");
+        }
+      );
+
+      let popupMenu = document.getElementById("fill-login-popup");
+      let firstLoginItem = popupMenu.getElementsByClassName(
+        "context-login-item"
+      )[0];
+      firstLoginItem.doCommand();
+
+      await passwordChangedPromise;
+
+      let contextMenu = document.getElementById("contentAreaContextMenu");
+      contextMenu.hidePopup();
+
+      // Blur the field to trigger a 'change' event.
+      await BrowserTestUtils.synthesizeKey("KEY_Tab", undefined, browser);
+      await BrowserTestUtils.synthesizeKey(
+        "KEY_Tab",
+        { shiftKey: true },
+        browser
+      );
+
+      await ContentTask.spawn(
+        browser,
+        [passwordInputSelector],
+        function checkFieldNotGeneratedPassword(inputSelector) {
+          let { LoginTestUtils: LTU } = ChromeUtils.import(
+            "resource://testing-common/LoginTestUtils.jsm"
+          );
+          const input = content.document.querySelector(inputSelector);
+          is(
+            input.value,
+            "pass1",
+            "Password field was filled with the saved password"
+          );
+          LTU.loginField.checkPasswordMasked(
+            input,
+            true,
+            "after fill of a saved login"
+          );
+        }
+      );
     }
   );
 
+  let logins = Services.logins.getAllLogins();
+  is(logins.length, 2, "Check 2 logins");
+  isnot(
+    logins[0].password,
+    logins[1].password,
+    "Generated password shouldn't have changed to match the filled password"
+  );
+
   Services.logins.removeAllLogins();
 });
 
 add_task(async function test_edited_generated_password_in_new_tab() {
   // test that we can fill the generated password into an empty password field,
   // edit it, and then fill the edited password.
   await BrowserTestUtils.withNewTab(
     {
--- a/toolkit/components/passwordmgr/test/browser/browser_context_menu_iframe.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_context_menu_iframe.js
@@ -31,39 +31,20 @@ add_task(async function test_initialize(
  */
 add_task(async function test_context_menu_iframe_fill() {
   await BrowserTestUtils.withNewTab(
     {
       gBrowser,
       url: TEST_ORIGIN + IFRAME_PAGE_PATH,
     },
     async function(browser) {
-      let contextMenuShownPromise = BrowserTestUtils.waitForEvent(
-        window,
-        "popupshown"
-      );
-      let eventDetails = { type: "contextmenu", button: 2 };
-
-      // To click at the right point we have to take into account the iframe offset.
-      // Synthesize a right mouse click over the password input element.
-      BrowserTestUtils.synthesizeMouseAtCenter(
-        ["#test-iframe", "#form-basic-password"],
-        eventDetails,
-        browser
-      );
-      await contextMenuShownPromise;
-
-      // Synthesize a mouse click over the fill login menu header.
-      let popupHeader = document.getElementById("fill-login");
-      let popupShownPromise = BrowserTestUtils.waitForEvent(
-        popupHeader,
-        "popupshown"
-      );
-      EventUtils.synthesizeMouseAtCenter(popupHeader, {});
-      await popupShownPromise;
+      await openPasswordContextMenu(browser, [
+        "#test-iframe",
+        "#form-basic-password",
+      ]);
 
       let popupMenu = document.getElementById("fill-login-popup");
 
       // Stores the original value of username
       function promiseFrameInputValue(name) {
         return ContentTask.spawn(browser, name, function(inputname) {
           let iframe = content.document.getElementById("test-iframe");
           let input = iframe.contentDocument.getElementById(inputname);
@@ -119,68 +100,55 @@ add_task(async function test_context_men
  */
 add_task(async function test_context_menu_iframe_sandbox() {
   await BrowserTestUtils.withNewTab(
     {
       gBrowser,
       url: TEST_ORIGIN + IFRAME_PAGE_PATH,
     },
     async function(browser) {
-      let contextMenuShownPromise = BrowserTestUtils.waitForEvent(
-        window,
-        "popupshown"
-      );
-      let eventDetails = { type: "contextmenu", button: 2 };
-
-      BrowserTestUtils.synthesizeMouseAtCenter(
+      await openPasswordContextMenu(
+        browser,
         ["#test-iframe-sandbox", "#form-basic-password"],
-        eventDetails,
-        browser
+        function checkDisabled() {
+          let popupHeader = document.getElementById("fill-login");
+          ok(
+            popupHeader.disabled,
+            "Check that the Fill Login menu item is disabled"
+          );
+          return false;
+        }
       );
-      await contextMenuShownPromise;
-
-      let popupHeader = document.getElementById("fill-login");
-      ok(
-        popupHeader.disabled,
-        "Check that the Fill Login menu item is disabled"
-      );
-
       let contextMenu = document.getElementById("contentAreaContextMenu");
       contextMenu.hidePopup();
     }
   );
 });
 
 /**
  * Check that the login context menu item appears for sandbox="allow-same-origin"
  */
 add_task(async function test_context_menu_iframe_sandbox_same_origin() {
   await BrowserTestUtils.withNewTab(
     {
       gBrowser,
       url: TEST_ORIGIN + IFRAME_PAGE_PATH,
     },
     async function(browser) {
-      let contextMenuShownPromise = BrowserTestUtils.waitForEvent(
-        window,
-        "popupshown"
-      );
-      let eventDetails = { type: "contextmenu", button: 2 };
-
-      BrowserTestUtils.synthesizeMouseAtCenter(
-        ["#test-iframe-sandbox-same-origin", "#form-basic-password"],
-        eventDetails,
-        browser
-      );
-      await contextMenuShownPromise;
-
-      let popupHeader = document.getElementById("fill-login");
-      ok(
-        !popupHeader.disabled,
-        "Check that the Fill Login menu item is enabled"
+      await openPasswordContextMenu(
+        browser,
+        ["#test-iframe-sandbox", "#form-basic-password"],
+        function checkDisabled() {
+          let popupHeader = document.getElementById("fill-login");
+          ok(
+            popupHeader.disabled,
+            "Check that the Fill Login menu item is disabled"
+          );
+          return false;
+        }
       );
 
       let contextMenu = document.getElementById("contentAreaContextMenu");
       contextMenu.hidePopup();
     }
   );
 });