Bug 1599752 - Partial back out of changeset c72321ba48b8 to remove subdomain suggestions in the context menu.

Differential Revision: https://phabricator.services.mozilla.com/D54937

--HG--
extra : moz-landing-system : lando

browser/base/content/nsContextMenu.js

@@ -954,14 +954,10 @@ class nsContextMenu {
     }
 
     let formOrigin = LoginHelper.getLoginOrigin(documentURI.spec);
-    let formActionOrigin = LoginHelper.getLoginOrigin(
-      loginFillInfo.formActionOrigin
-    );
     let fragment = nsContextMenu.LoginManagerContextMenu.addLoginsToMenu(
       this.targetIdentifier,
       this.browser,
-      formOrigin,
-      formActionOrigin
+      formOrigin
     );
     let isGeneratedPasswordEnabled =
       LoginHelper.generationAvailable && LoginHelper.generationEnabled;

browser/themes/linux/browser.css

@@ -458,15 +458,6 @@ notification[value="translation"] menulist > .menulist-dropmarker {
   margin-inline-end: 0 !important;
 }
 
-#fill-login-popup > menucaption {
-  color: GrayText;
-  font-weight: normal;
-}
-
-#fill-login-popup > menucaption > .menu-iconic-left {
-  display: none;
-}
-
 .webextension-popup-browser,
 .webextension-popup-stack {
   border-radius: inherit;

browser/themes/osx/browser.css

@@ -801,12 +801,6 @@ menulist.translate-infobar-element > .menulist-dropmarker {
   padding-right: 0;
 }
 
-#fill-login-popup > menucaption {
-  color: -moz-mac-menushadow;
-  font-weight: normal;
-  padding-inline-start: 11px;
-}
-
 .cui-widget-panelview[id^=PanelUI-webext-] {
   border-radius: 3.5px;
 }

browser/themes/windows/browser.css

@@ -832,19 +832,6 @@ panel[touchmode] .PanelUI-subView #appMenu-zoom-controls > .subviewbutton-iconic
   margin-top: -4px;
 }
 
-#fill-login-popup > menucaption {
-  color: GrayText;
-}
-
-/* Match the treatment of menucaption used for <optgroup> */
-#fill-login-popup > menucaption > .menu-iconic-left {
-  display: none
-}
-
-#fill-login-popup > menucaption > .menu-iconic-text {
-  -moz-appearance: menuitemtext
-}
-
 %include browser-aero.css
 
 @media (-moz-os-version: windows-win7) {

toolkit/components/passwordmgr/LoginAutoCompleteResult.jsm

@@ -65,8 +65,6 @@ XPCOMUtils.defineLazyGetter(this, "passwordMgrBundle", () => {
 function loginSort(formHostPort, a, b) {
   let maybeHostPortA = LoginHelper.maybeGetHostPortForURL(a.origin);
   let maybeHostPortB = LoginHelper.maybeGetHostPortForURL(b.origin);
-
-  // Exact hostPort matches should appear first.
   if (formHostPort == maybeHostPortA && formHostPort != maybeHostPortB) {
     return -1;
   }
@@ -84,8 +82,18 @@ function loginSort(formHostPort, a, b) {
     }
   }
 
-  // Finally sort by username.
-  return a.username.localeCompare(b.username);
+  let userA = a.username.toLowerCase();
+  let userB = b.username.toLowerCase();
+
+  if (userA < userB) {
+    return -1;
+  }
+
+  if (userA > userB) {
+    return 1;
+  }
+
+  return 0;
 }
 
 function findDuplicates(loginList) {

toolkit/components/passwordmgr/LoginManagerChild.jsm

@@ -2256,10 +2256,7 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
       usernameField = aField;
     }
 
-    let form = LoginFormFactory.createFromField(aField);
-
     return {
-      formActionOrigin: LoginHelper.getFormActionOrigin(form),
       usernameField: {
         found: !!usernameField,
         disabled:

toolkit/components/passwordmgr/LoginManagerContextMenu.jsm

@@ -40,17 +40,10 @@ this.LoginManagerContextMenu = {
    *        The origin of the document that the context menu was activated from.
    *        This isn't the same as the browser's top-level document origin
    *        when subframes are involved.
-   * @param {string} formActionOrigin
-   *        The origin of the LoginForm's action.
    * @returns {DocumentFragment} a document fragment with all the login items.
    */
-  addLoginsToMenu(
-    inputElementIdentifier,
-    browser,
-    formOrigin,
-    formActionOrigin
-  ) {
-    let foundLogins = this._findLogins(formOrigin, formActionOrigin);
+  addLoginsToMenu(inputElementIdentifier, browser, formOrigin) {
+    let foundLogins = this._findLogins(formOrigin);
 
     if (!foundLogins.length) {
       return null;
@@ -58,32 +51,7 @@ this.LoginManagerContextMenu = {
 
     let fragment = browser.ownerDocument.createDocumentFragment();
     let duplicateUsernames = this._findDuplicates(foundLogins);
-    // Default `lastDisplayOrigin` to the hostPort of the form so that we don't
-    // show a menucaption above logins that are direct matches for this document.
-    let lastDisplayOrigin = LoginHelper.maybeGetHostPortForURL(formOrigin);
-    let lastMenuCaption = null;
     for (let login of foundLogins) {
-      // Add a section header containing the displayOrigin above logins that
-      // aren't matches for the form's origin.
-      if (lastDisplayOrigin != login.displayOrigin) {
-        if (fragment.children.length) {
-          let menuSeparator = fragment.ownerDocument.createXULElement(
-            "menuseparator"
-          );
-          menuSeparator.className = "context-login-item";
-          fragment.appendChild(menuSeparator);
-        }
-
-        lastMenuCaption = fragment.ownerDocument.createXULElement(
-          "menucaption"
-        );
-        lastMenuCaption.setAttribute("role", "group");
-        lastMenuCaption.label = login.displayOrigin;
-        lastMenuCaption.className = "context-login-item";
-
-        fragment.appendChild(lastMenuCaption);
-      }
-
       let item = fragment.ownerDocument.createXULElement("menuitem");
 
       let username = login.username;
@@ -98,16 +66,8 @@ this.LoginManagerContextMenu = {
         );
         username = this._getLocalizedString("loginHostAge", [username, time]);
       }
-      item.id = "login-" + login.guid;
       item.setAttribute("label", username);
       item.setAttribute("class", "context-login-item");
-      if (lastMenuCaption) {
-        item.setAttribute("aria-level", "2");
-        lastMenuCaption.setAttribute(
-          "aria-owns",
-          lastMenuCaption.getAttribute("aria-owns") + item.id + " "
-        );
-      }
 
       // login is bound so we can keep the reference to each object.
       item.addEventListener(
@@ -123,7 +83,6 @@ this.LoginManagerContextMenu = {
       );
 
       fragment.appendChild(item);
-      lastDisplayOrigin = login.displayOrigin;
     }
 
     return fragment;
@@ -155,52 +114,51 @@ this.LoginManagerContextMenu = {
     });
   },
 
-  loginSort(formHostPort, a, b) {
-    let maybeHostPortA = LoginHelper.maybeGetHostPortForURL(a.origin);
-    let maybeHostPortB = LoginHelper.maybeGetHostPortForURL(b.origin);
-
-    // Exact hostPort matches should appear first.
-    if (formHostPort == maybeHostPortA && formHostPort != maybeHostPortB) {
-      return -1;
-    }
-    if (formHostPort != maybeHostPortA && formHostPort == maybeHostPortB) {
-      return 1;
-    }
-
-    // Next sort by displayOrigin (which contains the httpRealm)
-    if (a.displayOrigin !== b.displayOrigin) {
-      return a.displayOrigin.localeCompare(b.displayOrigin);
-    }
-
-    // Finally sort by username within the displayOrigin.
-    return a.username.localeCompare(b.username);
-  },
-
   /**
-   * Find logins for the specified origin.
+   * Find logins for the specified origin..
    *
    * @param {string} formOrigin
    *        Origin of the logins we want to find that has be sanitized by `getLoginOrigin`.
    *        This isn't the same as the browser's top-level document URI
    *        when subframes are involved.
-   * @param {string} formActionOrigin
    *
    * @returns {nsILoginInfo[]} a login list
    */
-  _findLogins(formOrigin, formActionOrigin) {
+  _findLogins(formOrigin) {
     let searchParams = {
-      acceptDifferentSubdomains: LoginHelper.includeOtherSubdomainsInLookup,
-      formActionOrigin,
-      ignoreActionAndRealm: true,
+      origin: formOrigin,
+      schemeUpgrades: LoginHelper.schemeUpgrades,
     };
-
-    let logins = LoginManagerParent.searchAndDedupeLogins(
-      formOrigin,
-      searchParams
+    let logins = LoginHelper.searchLoginsWithObject(searchParams);
+    let resolveBy = ["scheme", "timePasswordChanged"];
+    logins = LoginHelper.dedupeLogins(
+      logins,
+      ["username", "password"],
+      resolveBy,
+      formOrigin
     );
 
-    let formHostPort = LoginHelper.maybeGetHostPortForURL(formOrigin);
-    logins.sort(this.loginSort.bind(null, formHostPort));
+    // Sort logins in alphabetical order and by date.
+    logins.sort((loginA, loginB) => {
+      // Sort alphabetically
+      let result = loginA.username.localeCompare(loginB.username);
+      if (result) {
+        // Forces empty logins to be at the end
+        if (!loginA.username) {
+          return 1;
+        }
+        if (!loginB.username) {
+          return -1;
+        }
+        return result;
+      }
+
+      // Same username logins are sorted by last change date
+      let metaA = loginA.QueryInterface(Ci.nsILoginMetaInfo);
+      let metaB = loginB.QueryInterface(Ci.nsILoginMetaInfo);
+      return metaB.timePasswordChanged - metaA.timePasswordChanged;
+    });
+
     return logins;
   },
 

toolkit/components/passwordmgr/test/browser/browser_context_menu_generated_password.js

@@ -315,7 +315,7 @@ add_task(async function fill_generated_password_with_matching_logins() {
       let popupMenu = document.getElementById("fill-login-popup");
       let firstLoginItem = popupMenu.getElementsByClassName(
         "context-login-item"
-      )[1];
+      )[0];
       firstLoginItem.doCommand();
 
       await passwordChangedPromise;

toolkit/components/passwordmgr/test/unit/test_context_menu.js

@@ -15,7 +15,6 @@ const dateAndTimeFormatter = new Services.intl.DateTimeFormat(undefined, {
 const ORIGIN_HTTP_EXAMPLE_ORG = "http://example.org";
 const ORIGIN_HTTPS_EXAMPLE_ORG = "https://example.org";
 const ORIGIN_HTTPS_EXAMPLE_ORG_8080 = "https://example.org:8080";
-const ORIGIN_HTTPS_SUB_EXAMPLE_ORG = "https://sub.example.org";
 
 const FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1 = formLogin({
   formActionOrigin: ORIGIN_HTTPS_EXAMPLE_ORG,
@@ -47,21 +46,6 @@ const FORM_LOGIN_HTTPS_EXAMPLE_ORG_8080_U1_P2 = formLogin({
   password: "pass2",
 });
 
-// Subdomain
-
-const FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P1 = formLogin({
-  formActionOrigin: ORIGIN_HTTPS_SUB_EXAMPLE_ORG,
-  guid: "FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P1",
-  origin: ORIGIN_HTTPS_SUB_EXAMPLE_ORG,
-});
-
-const FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P2 = formLogin({
-  formActionOrigin: ORIGIN_HTTPS_SUB_EXAMPLE_ORG,
-  guid: "FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P2",
-  origin: ORIGIN_HTTPS_SUB_EXAMPLE_ORG,
-  password: "pass2",
-});
-
 // HTTP Auth.
 
 const HTTP_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1 = authLogin({
@@ -80,10 +64,9 @@ XPCOMUtils.defineLazyGetter(this, "_stringBundle", function() {
  */
 add_task(async function test_initialize() {
   Services.prefs.setBoolPref("signon.schemeUpgrades", true);
-  Services.prefs.setBoolPref("signon.includeOtherSubdomainsInLookup", true);
 });
 
-add_task(async function test_sameOriginOnlyHTTPS() {
+add_task(async function test_sameOriginBothHTTPAndHTTPSDeduped() {
   await runTestcase({
     formOrigin: FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1.origin,
     savedLogins: [FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1],
@@ -156,7 +139,7 @@ add_task(async function test_sameOriginSchemeDowngrade() {
   });
 });
 
-add_task(async function test_sameOriginShadowedSchemeUpgrade() {
+add_task(async function test_sameOriginNotShadowedSchemeUpgrade() {
   await runTestcase({
     formOrigin: FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1.origin,
     savedLogins: [
@@ -166,6 +149,11 @@ add_task(async function test_sameOriginShadowedSchemeUpgrade() {
     expectedItems: [
       {
         login: FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1,
+        time: true,
+      },
+      {
+        login: FORM_LOGIN_HTTP_EXAMPLE_ORG_U1_P2,
+        time: true,
       },
     ],
   });
@@ -199,13 +187,6 @@ add_task(async function test_sameDomainDifferentPort_onDefault() {
     expectedItems: [
       {
         login: FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1,
-        time: true,
-      },
-      "--", // separator
-      FORM_LOGIN_HTTPS_EXAMPLE_ORG_8080_U1_P2.displayOrigin, // group heading
-      {
-        login: FORM_LOGIN_HTTPS_EXAMPLE_ORG_8080_U1_P2,
-        time: true,
       },
     ],
   });
@@ -222,103 +203,6 @@ add_task(async function test_sameDomainDifferentPort_onNonDefault() {
     expectedItems: [
       {
         login: FORM_LOGIN_HTTPS_EXAMPLE_ORG_8080_U1_P2,
-        time: true,
-      },
-      "--", // separator
-      FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1.displayOrigin, // group heading
-      {
-        login: FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1,
-        time: true,
-      },
-    ],
-  });
-});
-
-// Subdomain tasks
-
-add_task(async function test_onlySubdomainOnBaseDomain() {
-  await runTestcase({
-    formOrigin: FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1.origin,
-    savedLogins: [FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P1],
-    expectedItems: [
-      // No separator
-      FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P1.displayOrigin,
-      {
-        login: FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P1,
-      },
-    ],
-  });
-});
-
-add_task(async function test_subdomainDedupeOnBaseDomain() {
-  await runTestcase({
-    formOrigin: FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1.origin,
-    savedLogins: [
-      FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1,
-      FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P1,
-    ],
-    expectedItems: [
-      {
-        login: FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1,
-      },
-    ],
-  });
-});
-
-add_task(async function test_subdomainDedupeOnSubDomain() {
-  await runTestcase({
-    formOrigin: FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P1.origin,
-    savedLogins: [
-      FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1,
-      FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P1,
-    ],
-    expectedItems: [
-      {
-        login: FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P1,
-      },
-    ],
-  });
-});
-
-add_task(async function test_subdomainIncludedOnBaseDomain() {
-  await runTestcase({
-    formOrigin: FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1.origin,
-    savedLogins: [
-      FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1,
-      FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P2,
-    ],
-    expectedItems: [
-      {
-        login: FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1,
-        time: true,
-      },
-      "--", // separator
-      FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P2.displayOrigin, // group heading
-      {
-        login: FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P2,
-        time: true,
-      },
-    ],
-  });
-});
-
-add_task(async function test_subdomainIncludedOnSubDomain() {
-  await runTestcase({
-    formOrigin: FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P2.origin,
-    savedLogins: [
-      FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1,
-      FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P2,
-    ],
-    expectedItems: [
-      {
-        login: FORM_LOGIN_HTTPS_SUB_EXAMPLE_ORG_U1_P2,
-        time: true,
-      },
-      "--", // separator
-      FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1.displayOrigin, // group heading
-      {
-        login: FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1,
-        time: true,
       },
     ],
   });
@@ -331,8 +215,6 @@ add_task(async function test_sameOriginOnlyHTTPAuth() {
     formOrigin: FORM_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1.origin,
     savedLogins: [HTTP_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1],
     expectedItems: [
-      // No separator
-      HTTP_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1.displayOrigin, // group heading
       {
         login: HTTP_LOGIN_HTTPS_EXAMPLE_ORG_U1_P1,
       },
@@ -394,7 +276,7 @@ async function runTestcase({ formOrigin, savedLogins, expectedItems }) {
   // Try to clear the fragment.
   LoginManagerContextMenu.clearLoginsFromMenu(document);
   Assert.equal(
-    document.querySelectorAll("menuitem, menuseparator, menucaption").length,
+    document.querySelectorAll("menuitem").length,
     0,
     "All items correctly cleared."
   );
@@ -432,24 +314,7 @@ function checkLoginItems(actualItems, expectedDetails) {
   for (let [i, expectedDetail] of expectedDetails.entries()) {
     let actualElement = actualItems[i];
 
-    // Separator
-    if (expectedDetail == "--") {
-      Assert.equal(actualElement.localName, "menuseparator", "Check localName");
-      continue;
-    }
-
-    // Section heading
-    if (typeof expectedDetail == "string") {
-      Assert.equal(actualElement.localName, "menucaption", "Check localName");
-      continue;
-    }
-
     Assert.equal(actualElement.localName, "menuitem", "Check localName");
-    Assert.equal(
-      actualElement.id,
-      "login-" + expectedDetail.login.guid,
-      `Check id ${i}`
-    );
 
     let expectedLabel = expectedDetail.login.username;
     if (!expectedLabel) {