Bug 1641413 - show all UNs from domain in the username field of the password manager doorhanger;r=MattN
authorSeverin <srudie@mozilla.com>
Fri, 17 Jul 2020 21:56:36 +0000
changeset 541009 1710b1714f8dcd0ebb189f3790d4db8918119f56
parent 541008 5bbc99899f4525ad98839b9b7277a4b26fe694f3
child 541010 0c0f777161a9499dd149853ff62d356f75d16c2a
push id37613
push userbtara@mozilla.com
push dateSat, 18 Jul 2020 09:26:25 +0000
treeherdermozilla-central@08cd64cdbc3b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1641413
milestone80.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 1641413 - show all UNs from domain in the username field of the password manager doorhanger;r=MattN Differential Revision: https://phabricator.services.mozilla.com/D80674
toolkit/components/passwordmgr/LoginHelper.jsm
toolkit/components/passwordmgr/LoginManagerPrompter.jsm
toolkit/components/passwordmgr/test/browser/browser_doorhanger_autocomplete_values.js
toolkit/components/passwordmgr/test/unit/test_LoginManagerPrompter_getUsernameSuggestions.js
--- a/toolkit/components/passwordmgr/LoginHelper.jsm
+++ b/toolkit/components/passwordmgr/LoginHelper.jsm
@@ -38,30 +38,34 @@ this.LoginHelper = {
   includeOtherSubdomainsInLookup: null,
   insecureAutofill: null,
   privateBrowsingCaptureEnabled: null,
   schemeUpgrades: null,
   showAutoCompleteFooter: null,
   showAutoCompleteImport: null,
   testOnlyUserHasInteractedWithDocument: null,
   userInputRequiredToCapture: null,
+  captureInputChanges: null,
 
   init() {
     // Watch for pref changes to update cached pref values.
     Services.prefs.addObserver("signon.", () => this.updateSignonPrefs());
     this.updateSignonPrefs();
     Services.telemetry.setEventRecordingEnabled("pwmgr", true);
     Services.telemetry.setEventRecordingEnabled("form_autocomplete", true);
   },
 
   updateSignonPrefs() {
     this.autofillForms = Services.prefs.getBoolPref("signon.autofillForms");
     this.autofillAutocompleteOff = Services.prefs.getBoolPref(
       "signon.autofillForms.autocompleteOff"
     );
+    this.captureInputChanges = Services.prefs.getBoolPref(
+      "signon.capture.inputChanges.enabled"
+    );
     this.debug = Services.prefs.getBoolPref("signon.debug");
     this.enabled = Services.prefs.getBoolPref("signon.rememberSignons");
     this.storageEnabled = Services.prefs.getBoolPref(
       "signon.storeSignons",
       true
     );
     this.formlessCaptureEnabled = Services.prefs.getBoolPref(
       "signon.formlessCapture.enabled"
--- a/toolkit/components/passwordmgr/LoginManagerPrompter.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerPrompter.jsm
@@ -621,30 +621,37 @@ class LoginManagerPrompter {
                 .addEventListener("keyup", onKeyUp);
               chromeDoc
                 .getElementById("password-notification-password")
                 .addEventListener("input", onPasswordInput);
               chromeDoc
                 .getElementById("password-notification-username-dropmarker")
                 .addEventListener("click", togglePopup);
 
-              let usernameSuggestions = LoginManagerPrompter._getUsernameSuggestions(
+              LoginManagerPrompter._getUsernameSuggestions(
                 login,
                 possibleValues?.usernames
-              );
-              chromeDoc.getElementById(
-                "password-notification-username-dropmarker"
-              ).hidden = !usernameSuggestions.length;
+              ).then(usernameSuggestions => {
+                let dropmarker = chromeDoc?.getElementById(
+                  "password-notification-username-dropmarker"
+                );
+                if (dropmarker) {
+                  dropmarker.hidden = !usernameSuggestions.length;
+                }
 
-              chromeDoc
-                .getElementById("password-notification-username")
-                .classList.toggle(
-                  "ac-has-end-icon",
-                  !!usernameSuggestions.length
+                let usernameField = chromeDoc?.getElementById(
+                  "password-notification-username"
                 );
+                if (usernameField) {
+                  usernameField.classList.toggle(
+                    "ac-has-end-icon",
+                    !!usernameSuggestions.length
+                  );
+                }
+              });
 
               let toggleBtn = chromeDoc.getElementById(
                 "password-notification-visibilityToggle"
               );
 
               if (
                 Services.prefs.getBoolPref(
                   "signon.rememberSignons.visibilityToggle"
@@ -965,23 +972,26 @@ class LoginManagerPrompter {
   }
 
   /**
    * Set the values that will be used the next time the username autocomplete popup is opened.
    *
    * @param {nsILoginInfo} login - used only for its information about the current domain.
    * @param {Set<String>?} possibleUsernames - values that we believe may be new/changed login usernames.
    */
-  static _setUsernameAutocomplete(login, possibleUsernames = new Set()) {
+  static async _setUsernameAutocomplete(login, possibleUsernames = new Set()) {
     let result = Cc["@mozilla.org/autocomplete/simple-result;1"].createInstance(
       Ci.nsIAutoCompleteSimpleResult
     );
     result.setDefaultIndex(0);
 
-    let usernames = this._getUsernameSuggestions(login, possibleUsernames);
+    let usernames = await this._getUsernameSuggestions(
+      login,
+      possibleUsernames
+    );
     for (let { text, style } of usernames) {
       let value = text;
       let comment = "";
       let image = "";
       let _style = style;
       result.appendMatch(value, comment, image, _style);
     }
 
@@ -995,44 +1005,47 @@ class LoginManagerPrompter {
   }
 
   /**
    * @param {nsILoginInfo} login - used only for its information about the current domain.
    * @param {Set<String>?} possibleUsernames - values that we believe may be new/changed login usernames.
    *
    * @returns {object[]} an ordered list of usernames to be used the next time the username autocomplete popup is opened.
    */
-  static _getUsernameSuggestions(login, possibleUsernames = new Set()) {
-    // TODO uncomment in bug 1641413
-    // let sameOriginLogins = LoginHelper.searchLoginsWithObject({
-    //   formActionOrigin: login.formActionOrigin,
-    //   origin: login.origin,
-    //   httpRealm: login.httpRealm,
-    //   schemeUpgrades: LoginHelper.schemeUpgrades,
-    // });
+  static async _getUsernameSuggestions(login, possibleUsernames = new Set()) {
+    if (!Services.prefs.getBoolPref("signon.capture.inputChanges.enabled")) {
+      return [];
+    }
 
-    // let saved = sameOriginLogins.map(login => {
-    //   return { text: login.username, style: "login" };
-    // });
-    let possible = [...possibleUsernames].map(username => {
-      // TODO style will be "possible-username" in bug 1641413
-      return { text: username, style: "" };
+    let baseDomainLogins = await Services.logins.searchLoginsAsync({
+      origin: login.origin,
+      schemeUpgrades: LoginHelper.schemeUpgrades,
     });
 
-    // TODO concat with `saved` in bug 1641413
-    return possible.reduce((acc, next) => {
-      let alreadyInAcc = acc.findIndex(entry => entry.text == next.text) != -1;
-      if (!alreadyInAcc) {
-        acc.push(next);
-      } else if (next.style == "possible-username") {
-        let existingIndex = acc.findIndex(entry => entry.text == next.text);
-        acc[existingIndex] = next;
-      }
-      return acc;
-    }, []);
+    let saved = baseDomainLogins.map(login => {
+      return { text: login.username, style: "login" };
+    });
+    let possible = [...possibleUsernames].map(username => {
+      return { text: username, style: "possible-username" };
+    });
+
+    return possible
+      .concat(saved)
+      .reduce((acc, next) => {
+        let alreadyInAcc =
+          acc.findIndex(entry => entry.text == next.text) != -1;
+        if (!alreadyInAcc) {
+          acc.push(next);
+        } else if (next.style == "possible-username") {
+          let existingIndex = acc.findIndex(entry => entry.text == next.text);
+          acc[existingIndex] = next;
+        }
+        return acc;
+      }, [])
+      .filter(suggestion => !!suggestion.text);
   }
 }
 
 XPCOMUtils.defineLazyGetter(this, "log", () => {
   return LoginHelper.createLogger("LoginManagerPrompter");
 });
 
 const EXPORTED_SYMBOLS = ["LoginManagerPrompter"];
--- a/toolkit/components/passwordmgr/test/browser/browser_doorhanger_autocomplete_values.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_doorhanger_autocomplete_values.js
@@ -67,36 +67,35 @@ const TEST_CASES = [
     modifiedFields: [
       { [PASSWORD_SELECTOR]: "myPassword" },
       { [USERNAME_SELECTOR]: "new_username" },
       { [USERNAME_SELECTOR]: "" },
     ],
     expectUsernameDropmarker: true,
     expectedValues: ["new_username"],
   },
-  // TODO uncomment in bug 1641413
-  // {
-  //   description: "saved logins should be displayed in popup",
-  //   modifiedFields: [
-  //     { [PASSWORD_SELECTOR]: "myPassword" },
-  //     { [USERNAME_SELECTOR]: "new_username" },
-  //   ],
-  //   savedLogins: [
-  //     {
-  //       username: "savedUn1",
-  //       password: "somePass",
-  //     },
-  //     {
-  //       username: "savedUn2",
-  //       password: "otherPass",
-  //     },
-  //   ],
-  //   expectUsernameDropmarker: true,
-  //   expectedValues: ["new_username", "savedUn1", "savedUn2"],
-  // },
+  {
+    description: "saved logins should be displayed in popup",
+    modifiedFields: [
+      { [USERNAME_SELECTOR]: "new_username" },
+      { [PASSWORD_SELECTOR]: "myPassword" },
+    ],
+    savedLogins: [
+      {
+        username: "savedUn1",
+        password: "somePass",
+      },
+      {
+        username: "savedUn2",
+        password: "otherPass",
+      },
+    ],
+    expectUsernameDropmarker: true,
+    expectedValues: ["new_username", "savedUn1", "savedUn2"],
+  },
   {
     description: "duplicated page usernames should only be displayed once",
     modifiedFields: [
       { [PASSWORD_SELECTOR]: "myPassword" },
       { [USERNAME_SELECTOR]: "new_username1" },
       { [USERNAME_SELECTOR]: "new_username2" },
       { [USERNAME_SELECTOR]: "new_username1" },
     ],
@@ -158,17 +157,16 @@ add_task(async function test_edit_passwo
 
     // Clean state before the test case is executed.
     await LoginTestUtils.clearData();
     await cleanupDoorhanger();
     await cleanupPasswordNotifications();
     Services.logins.removeAllLogins();
 
     // Create the pre-existing logins when needed.
-    // TODO will not be used until bug 1641413
     info("Adding any saved logins");
     if (testCase.savedLogins) {
       for (let login of testCase.savedLogins) {
         info("Adding login " + JSON.stringify(login));
         _addSavedLogin(login.username);
       }
     }
 
--- a/toolkit/components/passwordmgr/test/unit/test_LoginManagerPrompter_getUsernameSuggestions.js
+++ b/toolkit/components/passwordmgr/test/unit/test_LoginManagerPrompter_getUsernameSuggestions.js
@@ -1,58 +1,85 @@
 let { LoginManagerPrompter } = ChromeUtils.import(
   "resource://gre/modules/LoginManagerPrompter.jsm"
 );
 
 const TEST_CASES = [
-  // TODO uncomment in bug 1641413
-  //   {
-  //     description: "page values should appear before saved values",
-  //     savedLogins: [{ username: "savedUsername", password: "savedPassword" }],
-  //     possibleUsernames: ["pageUsername"],
-  //     expectedSuggestions: [
-  //       { text: "pageUsername", style: "possible-username" },
-  //       { text: "savedUsername", style: "login" },
-  //     ],
-  //   },
+  {
+    description: "page values should appear before saved values",
+    savedLogins: [{ username: "savedUsername", password: "savedPassword" }],
+    possibleUsernames: ["pageUsername"],
+    expectedSuggestions: [
+      { text: "pageUsername", style: "possible-username" },
+      { text: "savedUsername", style: "login" },
+    ],
+  },
   {
     description: "duplicate page values should be deduped",
     savedLogins: [],
     possibleUsernames: ["pageUsername", "pageUsername", "pageUsername2"],
     expectedSuggestions: [
-      { text: "pageUsername", style: "" },
-      { text: "pageUsername2", style: "" },
+      { text: "pageUsername", style: "possible-username" },
+      { text: "pageUsername2", style: "possible-username" },
     ],
   },
-  // TODO uncomment in bug 1641413
-  //   {
-  //     description: "page values should dedupe and win over saved values",
-  //     savedLogins: [{ username: "username", password: "savedPassword" }],
-  //     possibleUsernames: ["username"],
-  //     expectedSuggestions: [{ text: "username", style: "possible-username" }],
-  //   },
+  {
+    description: "page values should dedupe and win over saved values",
+    savedLogins: [{ username: "username", password: "savedPassword" }],
+    possibleUsernames: ["username"],
+    expectedSuggestions: [{ text: "username", style: "possible-username" }],
+  },
+  {
+    description: "empty usernames should be filtered out",
+    savedLogins: [{ username: "", password: "savedPassword" }],
+    possibleUsernames: [""],
+    expectedSuggestions: [],
+  },
+  {
+    description: "auth logins should be displayed alongside normal ones",
+    savedLogins: [
+      { username: "normalUsername", password: "normalPassword" },
+      { isAuth: true, username: "authUsername", password: "authPassword" },
+    ],
+    possibleUsernames: [""],
+    expectedSuggestions: [
+      { text: "normalUsername", style: "login" },
+      { text: "authUsername", style: "login" },
+    ],
+  },
 ];
 
-const LOGIN = LoginTestUtils.testData.formLogin({
+const LOGIN = TestData.formLogin({
   origin: "https://example.com",
   formActionOrigin: "https://example.com",
   username: "LOGIN is used only for its origin",
   password: "LOGIN is used only for its origin",
 });
 
 function _saveLogins(logins) {
   logins
-    .map(loginData =>
-      LoginTestUtils.testData.formLogin({
-        origin: "https://example.com",
-        formActionOrigin: "https://example.com",
-        username: loginData.username,
-        password: loginData.password,
-      })
-    )
+    .map(loginData => {
+      let login;
+      if (loginData.isAuth) {
+        login = TestData.authLogin({
+          origin: "https://example.com",
+          httpRealm: "example-realm",
+          username: loginData.username,
+          password: loginData.password,
+        });
+      } else {
+        login = TestData.formLogin({
+          origin: "https://example.com",
+          formActionOrigin: "https://example.com",
+          username: loginData.username,
+          password: loginData.password,
+        });
+      }
+      return login;
+    })
     .forEach(login => Services.logins.addLogin(login));
 }
 
 function _compare(expectedArr, actualArr) {
   ok(!!expectedArr, "Expect expectedArr to be truthy");
   ok(!!actualArr, "Expect actualArr to be truthy");
   ok(
     expectedArr.length == actualArr.length,
@@ -68,30 +95,30 @@ function _compare(expectedArr, actualArr
     );
     ok(
       expected.style == actual.style,
       `Expect element #${i} text to match.  Expected: '${expected.style}', Actual '${actual.style}'`
     );
   }
 }
 
-function _test(testCase) {
+async function _test(testCase) {
   info(`Starting test case: ${testCase.description}`);
   info(`Storing saved logins: ${JSON.stringify(testCase.savedLogins)}`);
   _saveLogins(testCase.savedLogins);
 
   info("Computing results");
-  let result = LoginManagerPrompter._getUsernameSuggestions(
+  let result = await LoginManagerPrompter._getUsernameSuggestions(
     LOGIN,
     testCase.possibleUsernames
   );
 
   _compare(testCase.expectedSuggestions, result);
 
   info("Cleaning up state");
   LoginTestUtils.clearData();
 }
 
-add_task(function test_LoginManagerPrompter_getUsernameSuggestions() {
+add_task(async function test_LoginManagerPrompter_getUsernameSuggestions() {
   for (let tc of TEST_CASES) {
-    _test(tc);
+    await _test(tc);
   }
 });