diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js index 8188ba5cdb9f..c161653ebca7 100644 --- a/browser/base/content/nsContextMenu.js +++ b/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; diff --git a/browser/themes/linux/browser.css b/browser/themes/linux/browser.css index 446a53feb29a..7579028a1d1a 100644 --- a/browser/themes/linux/browser.css +++ b/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; diff --git a/browser/themes/osx/browser.css b/browser/themes/osx/browser.css index c1f924ceb69e..23abadf84bf9 100644 --- a/browser/themes/osx/browser.css +++ b/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; } diff --git a/browser/themes/windows/browser.css b/browser/themes/windows/browser.css index 3ec85970581f..3dbb5263aa9d 100644 --- a/browser/themes/windows/browser.css +++ b/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 */ -#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) { diff --git a/toolkit/components/passwordmgr/LoginAutoCompleteResult.jsm b/toolkit/components/passwordmgr/LoginAutoCompleteResult.jsm index 042599e7c819..21ef27b3a27b 100644 --- a/toolkit/components/passwordmgr/LoginAutoCompleteResult.jsm +++ b/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) { diff --git a/toolkit/components/passwordmgr/LoginManagerChild.jsm b/toolkit/components/passwordmgr/LoginManagerChild.jsm index e2a229c151ca..a20e74038332 100644 --- a/toolkit/components/passwordmgr/LoginManagerChild.jsm +++ b/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: diff --git a/toolkit/components/passwordmgr/LoginManagerContextMenu.jsm b/toolkit/components/passwordmgr/LoginManagerContextMenu.jsm index e1951fcf5954..3002d5236158 100644 --- a/toolkit/components/passwordmgr/LoginManagerContextMenu.jsm +++ b/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; }, diff --git a/toolkit/components/passwordmgr/test/browser/browser_context_menu_generated_password.js b/toolkit/components/passwordmgr/test/browser/browser_context_menu_generated_password.js index 01bb3632b0a3..4f8b06c5c539 100644 --- a/toolkit/components/passwordmgr/test/browser/browser_context_menu_generated_password.js +++ b/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; diff --git a/toolkit/components/passwordmgr/test/unit/test_context_menu.js b/toolkit/components/passwordmgr/test/unit/test_context_menu.js index 82d98a750cfc..7340c686ada5 100644 --- a/toolkit/components/passwordmgr/test/unit/test_context_menu.js +++ b/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) {