From 42a79e65cf3581b92894bbc395678024bd461d5d Mon Sep 17 00:00:00 2001 From: Daniel Riera Date: Thu, 13 Nov 2025 10:26:32 -0500 Subject: [PATCH] [PM-26916] inline menu not autofilling email field for oatsovernight.com (#17182) * PM-26916 utilize opid on focused fields as first validation in order to avoid erroneously filling other similar fields * extract logic to helper and take totp and multiple forms into account * run prettier * avoid filling with opid if already filled * clean up comments and avoid early return so all fields are scanned * add tests --- .../abstractions/overlay.background.ts | 1 + .../autofill/background/overlay.background.ts | 2 + .../services/abstractions/autofill.service.ts | 2 + .../autofill-overlay-content.service.ts | 1 + .../services/autofill.service.spec.ts | 142 ++++++++++++++++++ .../src/autofill/services/autofill.service.ts | 133 ++++++++++++---- 6 files changed, 254 insertions(+), 27 deletions(-) diff --git a/apps/browser/src/autofill/background/abstractions/overlay.background.ts b/apps/browser/src/autofill/background/abstractions/overlay.background.ts index 18cf1d2044..96809fa26b 100644 --- a/apps/browser/src/autofill/background/abstractions/overlay.background.ts +++ b/apps/browser/src/autofill/background/abstractions/overlay.background.ts @@ -47,6 +47,7 @@ export type FocusedFieldData = { accountCreationFieldType?: string; showPasskeys?: boolean; focusedFieldForm?: string; + focusedFieldOpid?: string; }; export type InlineMenuElementPosition = { diff --git a/apps/browser/src/autofill/background/overlay.background.ts b/apps/browser/src/autofill/background/overlay.background.ts index 35585d5886..f3278fa6b0 100644 --- a/apps/browser/src/autofill/background/overlay.background.ts +++ b/apps/browser/src/autofill/background/overlay.background.ts @@ -1176,6 +1176,7 @@ export class OverlayBackground implements OverlayBackgroundInterface { fillNewPassword: true, allowTotpAutofill: true, focusedFieldForm: this.focusedFieldData?.focusedFieldForm, + focusedFieldOpid: this.focusedFieldData?.focusedFieldOpid, }); if (totpCode) { @@ -1861,6 +1862,7 @@ export class OverlayBackground implements OverlayBackgroundInterface { fillNewPassword: true, allowTotpAutofill: false, focusedFieldForm: this.focusedFieldData?.focusedFieldForm, + focusedFieldOpid: this.focusedFieldData?.focusedFieldOpid, }); globalThis.setTimeout(async () => { diff --git a/apps/browser/src/autofill/services/abstractions/autofill.service.ts b/apps/browser/src/autofill/services/abstractions/autofill.service.ts index 13a00fb573..85bf8c1661 100644 --- a/apps/browser/src/autofill/services/abstractions/autofill.service.ts +++ b/apps/browser/src/autofill/services/abstractions/autofill.service.ts @@ -29,6 +29,7 @@ export interface AutoFillOptions { allowTotpAutofill?: boolean; autoSubmitLogin?: boolean; focusedFieldForm?: string; + focusedFieldOpid?: string; } export interface FormData { @@ -47,6 +48,7 @@ export interface GenerateFillScriptOptions { cipher: CipherView; tabUrl: string; defaultUriMatch: UriMatchStrategySetting; + focusedFieldOpid?: string; } export type CollectPageDetailsResponseMessage = { diff --git a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts index fd4d9b6080..7854dc8e16 100644 --- a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts +++ b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts @@ -975,6 +975,7 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ showPasskeys: !!autofillFieldData?.showPasskeys, accountCreationFieldType: autofillFieldData?.accountCreationFieldType, focusedFieldForm: autofillFieldData?.form, + focusedFieldOpid: autofillFieldData?.opid, }; const allFields = this.formFieldElements; diff --git a/apps/browser/src/autofill/services/autofill.service.spec.ts b/apps/browser/src/autofill/services/autofill.service.spec.ts index bfeaa360a3..b436214f32 100644 --- a/apps/browser/src/autofill/services/autofill.service.spec.ts +++ b/apps/browser/src/autofill/services/autofill.service.spec.ts @@ -50,6 +50,7 @@ import AutofillPageDetails from "../models/autofill-page-details"; import AutofillScript from "../models/autofill-script"; import { createAutofillFieldMock, + createAutofillFormMock, createAutofillPageDetailsMock, createAutofillScriptMock, createChromeTabMock, @@ -2309,6 +2310,147 @@ describe("AutofillService", () => { untrustedIframe: false, }); }); + + describe("given a focused username field", () => { + let focusedField: AutofillField; + let passwordField: AutofillField; + + beforeEach(() => { + focusedField = createAutofillFieldMock({ + opid: "focused-username", + type: "text", + form: "form1", + elementNumber: 1, + }); + passwordField = createAutofillFieldMock({ + opid: "password", + type: "password", + form: "form1", + elementNumber: 2, + }); + pageDetails.forms = { + form1: createAutofillFormMock({ opid: "form1" }), + }; + options.focusedFieldOpid = "focused-username"; + jest.spyOn(autofillService as any, "inUntrustedIframe").mockResolvedValue(false); + jest.spyOn(AutofillService, "fillByOpid"); + }); + + it("will return early when no matching password is found and set autosubmit if enabled", async () => { + pageDetails.fields = [focusedField]; + options.autoSubmitLogin = true; + + const value = await autofillService["generateLoginFillScript"]( + fillScript, + pageDetails, + filledFields, + options, + ); + + expect(AutofillService.fillByOpid).toHaveBeenCalledTimes(1); + expect(AutofillService.fillByOpid).toHaveBeenCalledWith( + fillScript, + focusedField, + options.cipher.login.username, + ); + expect(value.autosubmit).toEqual(["form1"]); + }); + + it("will prioritize focused field and skip passwords in different forms", async () => { + const otherUsername = createAutofillFieldMock({ + opid: "other-username", + type: "text", + form: "form1", + elementNumber: 2, + }); + const passwordDifferentForm = createAutofillFieldMock({ + opid: "password-different", + type: "password", + form: "form2", + elementNumber: 1, + }); + pageDetails.fields = [focusedField, otherUsername, passwordField, passwordDifferentForm]; + pageDetails.forms.form2 = createAutofillFormMock({ opid: "form2" }); + + await autofillService["generateLoginFillScript"]( + fillScript, + pageDetails, + filledFields, + options, + ); + + expect(AutofillService.fillByOpid).toHaveBeenCalledWith( + fillScript, + focusedField, + options.cipher.login.username, + ); + expect(AutofillService.fillByOpid).toHaveBeenCalledWith( + fillScript, + passwordField, + options.cipher.login.password, + ); + expect(AutofillService.fillByOpid).not.toHaveBeenCalledWith( + fillScript, + otherUsername, + expect.anything(), + ); + expect(AutofillService.fillByOpid).not.toHaveBeenCalledWith( + fillScript, + passwordDifferentForm, + expect.anything(), + ); + }); + + it("will not fill focused field if already in filledFields", async () => { + pageDetails.fields = [focusedField, passwordField]; + filledFields[focusedField.opid] = focusedField; + + await autofillService["generateLoginFillScript"]( + fillScript, + pageDetails, + filledFields, + options, + ); + + expect(AutofillService.fillByOpid).not.toHaveBeenCalledWith( + fillScript, + focusedField, + expect.anything(), + ); + expect(AutofillService.fillByOpid).toHaveBeenCalledWith( + fillScript, + passwordField, + options.cipher.login.password, + ); + }); + + it.each([ + ["email", "email"], + ["tel", "tel"], + ])("will treat focused %s field as username field", async (type, opid) => { + const focusedTypedField = createAutofillFieldMock({ + opid: `focused-${opid}`, + type: type as "email" | "tel", + form: "form1", + elementNumber: 1, + }); + pageDetails.fields = [focusedTypedField, passwordField]; + options.focusedFieldOpid = `focused-${opid}`; + + await autofillService["generateLoginFillScript"]( + fillScript, + pageDetails, + filledFields, + options, + ); + + expect(AutofillService.fillByOpid).toHaveBeenCalledWith( + fillScript, + focusedTypedField, + options.cipher.login.username, + ); + }); + }); }); }); diff --git a/apps/browser/src/autofill/services/autofill.service.ts b/apps/browser/src/autofill/services/autofill.service.ts index f5df17083c..fcc8861228 100644 --- a/apps/browser/src/autofill/services/autofill.service.ts +++ b/apps/browser/src/autofill/services/autofill.service.ts @@ -451,6 +451,7 @@ export default class AutofillService implements AutofillServiceInterface { cipher: options.cipher, tabUrl: tab.url, defaultUriMatch: defaultUriMatch, + focusedFieldOpid: options.focusedFieldOpid, }); if (!fillScript || !fillScript.script || !fillScript.script.length) { @@ -837,7 +838,7 @@ export default class AutofillService implements AutofillServiceInterface { } const passwords: AutofillField[] = []; - const usernames: AutofillField[] = []; + const usernames = new Map(); const totps: AutofillField[] = []; let pf: AutofillField = null; let username: AutofillField = null; @@ -871,6 +872,70 @@ export default class AutofillService implements AutofillServiceInterface { const prioritizedPasswordFields = loginPasswordFields.length > 0 ? loginPasswordFields : registrationPasswordFields; + const focusedField = + options.focusedFieldOpid && + pageDetails.fields.find((f) => f.opid === options.focusedFieldOpid); + const focusedForm = focusedField?.form; + + const isFocusedTotpField = + focusedField && + options.allowTotpAutofill && + (focusedField.type === "text" || + focusedField.type === "number" || + focusedField.type === "tel") && + (AutofillService.fieldIsFuzzyMatch(focusedField, [ + ...AutoFillConstants.TotpFieldNames, + ...AutoFillConstants.AmbiguousTotpFieldNames, + ]) || + focusedField.autoCompleteType === "one-time-code") && + !AutofillService.fieldIsFuzzyMatch(focusedField, [ + ...AutoFillConstants.RecoveryCodeFieldNames, + ]); + + const focusedUsernameField = + focusedField && + !isFocusedTotpField && + login.username && + (focusedField.type === "text" || + focusedField.type === "email" || + focusedField.type === "tel") && + focusedField; + + const passwordMatchesFocused = (pf: AutofillField): boolean => + !focusedField + ? true + : focusedForm != null + ? pf.form === focusedForm + : focusedUsernameField && + pf.form == null && + this.findUsernameField(pageDetails, pf, false, false, true)?.opid === + focusedUsernameField.opid; + + const getUsernameForPassword = ( + pf: AutofillField, + withoutForm: boolean, + ): AutofillField | null => { + // use focused username if it matches this password, otherwise fall back to finding username field before password + if (focusedUsernameField && passwordMatchesFocused(pf)) { + return focusedUsernameField; + } + return this.findUsernameField(pageDetails, pf, false, false, withoutForm); + }; + + if (focusedUsernameField && !prioritizedPasswordFields.some(passwordMatchesFocused)) { + if (!Object.prototype.hasOwnProperty.call(filledFields, focusedUsernameField.opid)) { + filledFields[focusedUsernameField.opid] = focusedUsernameField; + AutofillService.fillByOpid(fillScript, focusedUsernameField, login.username); + if (options.autoSubmitLogin && focusedUsernameField.form) { + fillScript.autosubmit = [focusedUsernameField.form]; + } + return AutofillService.setFillScriptForFocus( + { [focusedUsernameField.opid]: focusedUsernameField }, + fillScript, + ); + } + } + for (const formKey in pageDetails.forms) { // eslint-disable-next-line if (!pageDetails.forms.hasOwnProperty(formKey)) { @@ -878,20 +943,25 @@ export default class AutofillService implements AutofillServiceInterface { } prioritizedPasswordFields.forEach((passField) => { + if (focusedField && !passwordMatchesFocused(passField)) { + return; + } + pf = passField; passwords.push(pf); if (login.username) { - username = this.findUsernameField(pageDetails, pf, false, false, false); - + username = getUsernameForPassword(pf, false); if (username) { - usernames.push(username); + usernames.set(username.opid, username); } } if (options.allowTotpAutofill && login.totp) { - totp = this.findTotpField(pageDetails, pf, false, false, false); - + totp = + isFocusedTotpField && passwordMatchesFocused(passField) + ? focusedField + : this.findTotpField(pageDetails, pf, false, false, false); if (totp) { totps.push(totp); } @@ -900,24 +970,30 @@ export default class AutofillService implements AutofillServiceInterface { } if (passwordFields.length && !passwords.length) { - // The page does not have any forms with password fields. Use the first password field on the page and the - // input field just before it as the username. - pf = prioritizedPasswordFields[0]; - passwords.push(pf); + // in the event that password fields exist but weren't processed within form elements. + // select matching password if focused, otherwise first in prioritized list. for username, use focused field if it matches, otherwise find field before password. + const passwordFieldToUse = focusedField + ? prioritizedPasswordFields.find(passwordMatchesFocused) || prioritizedPasswordFields[0] + : prioritizedPasswordFields[0]; - if (login.username && pf.elementNumber > 0) { - username = this.findUsernameField(pageDetails, pf, false, false, true); + if (passwordFieldToUse) { + passwords.push(passwordFieldToUse); - if (username) { - usernames.push(username); + if (login.username && passwordFieldToUse.elementNumber > 0) { + username = getUsernameForPassword(passwordFieldToUse, true); + if (username) { + usernames.set(username.opid, username); + } } - } - if (options.allowTotpAutofill && login.totp && pf.elementNumber > 0) { - totp = this.findTotpField(pageDetails, pf, false, false, true); - - if (totp) { - totps.push(totp); + if (options.allowTotpAutofill && login.totp && passwordFieldToUse.elementNumber > 0) { + totp = + isFocusedTotpField && passwordMatchesFocused(passwordFieldToUse) + ? focusedField + : this.findTotpField(pageDetails, passwordFieldToUse, false, false, true); + if (totp) { + totps.push(totp); + } } } } @@ -951,7 +1027,7 @@ export default class AutofillService implements AutofillServiceInterface { totps.push(field); return; case isFillableUsernameField: - usernames.push(field); + usernames.set(field.opid, field); return; default: return; @@ -960,9 +1036,10 @@ export default class AutofillService implements AutofillServiceInterface { } const formElementsSet = new Set(); - usernames.forEach((u) => { - // eslint-disable-next-line - if (filledFields.hasOwnProperty(u.opid)) { + const usernamesToFill = focusedUsernameField ? [focusedUsernameField] : [...usernames.values()]; + + usernamesToFill.forEach((u) => { + if (Object.prototype.hasOwnProperty.call(filledFields, u.opid)) { return; } @@ -2330,12 +2407,14 @@ export default class AutofillService implements AutofillServiceInterface { const includesUsernameFieldName = this.findMatchingFieldIndex(f, AutoFillConstants.UsernameFieldNames) > -1; - const isInSameForm = f.form === passwordField.form; + // only consider fields in same form if both have non-null form values + // null forms are treated as separate + const isInSameForm = + f.form != null && passwordField.form != null && f.form === passwordField.form; // An email or tel field in the same form as the password field is likely a qualified // candidate for autofill, even if visibility checks are unreliable - const isQualifiedUsernameField = - f.form === passwordField.form && (f.type === "email" || f.type === "tel"); + const isQualifiedUsernameField = isInSameForm && (f.type === "email" || f.type === "tel"); if ( !f.disabled &&