[PM-28289] Address false-positives of new login save prompts (#17783)

* add values to TotpFieldNames constant

* add totp field check to username field qualification

* handle checking empty string cases

* update tests

* require stored username for new cipher notification prompt

* drop ambiguous token keyword from authoritative TOTP field names constant

* adjust shouldAttemptNotification logic for add and change cases

apps/browser/src/autofill/background/overlay-notifications.background.ts

@@ -262,11 +262,30 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg
    */
   private notificationDataIncompleteOnBeforeRequest = (tabId: number) => {
     const modifyLoginData = this.modifyLoginCipherFormData.get(tabId);
-    return (
-      !modifyLoginData ||
-      !this.shouldAttemptNotification(modifyLoginData, NotificationTypes.Add) ||
-      !this.shouldAttemptNotification(modifyLoginData, NotificationTypes.Change)
+
+    if (!modifyLoginData) {
+      return true;
+    }
+
+    const shouldAttemptAddNotification = this.shouldAttemptNotification(
+      modifyLoginData,
+      NotificationTypes.Add,
     );
+
+    if (shouldAttemptAddNotification) {
+      return false;
+    }
+
+    const shouldAttemptChangeNotification = this.shouldAttemptNotification(
+      modifyLoginData,
+      NotificationTypes.Change,
+    );
+
+    if (shouldAttemptChangeNotification) {
+      return false;
+    }
+
+    return false;
   };
 
   /**
@@ -454,15 +473,27 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg
     modifyLoginData: ModifyLoginCipherFormData,
     notificationType: NotificationType,
   ): boolean => {
+    // Intentionally not stripping whitespace characters here as they
+    // represent user entry.
+    const usernameFieldHasValue = !!(modifyLoginData?.username || "").length;
+    const passwordFieldHasValue = !!(modifyLoginData?.password || "").length;
+    const newPasswordFieldHasValue = !!(modifyLoginData?.newPassword || "").length;
+
+    const canBeUserLogin = usernameFieldHasValue && passwordFieldHasValue;
+    const canBePasswordUpdate = passwordFieldHasValue && newPasswordFieldHasValue;
+
     switch (notificationType) {
+      // `Add` case included because all forms with cached usernames (from previous
+      // visits) will appear to be "password only" and otherwise trigger the new login
+      // save notification.
       case NotificationTypes.Add:
-        return (
-          modifyLoginData?.username && !!(modifyLoginData.password || modifyLoginData.newPassword)
-        );
+        // Can be values for nonstored login or account creation
+        return usernameFieldHasValue && (passwordFieldHasValue || newPasswordFieldHasValue);
       case NotificationTypes.Change:
-        return !!(modifyLoginData.password || modifyLoginData.newPassword);
+        // Can be login with nonstored login changes or account password update
+        return canBeUserLogin || canBePasswordUpdate;
       case NotificationTypes.AtRiskPassword:
-        return !modifyLoginData.newPassword;
+        return !newPasswordFieldHasValue;
       case NotificationTypes.Unlock:
         // Unlock notifications are handled separately and do not require form data
         return false;

apps/browser/src/autofill/services/autofill-constants.ts

@@ -39,6 +39,7 @@ export class AutoFillConstants {
     "otpcode",
     "onetimepassword",
     "security_code",
+    "second-factor",
     "twofactor",
     "twofa",
     "twofactorcode",

apps/browser/src/autofill/services/autofill-overlay-content.service.spec.ts

@@ -1603,14 +1603,14 @@ describe("AutofillOverlayContentService", () => {
 
         it("skips triggering submission if a button is not found", async () => {
           const submitButton = document.querySelector("button");
-          submitButton.remove();
+          submitButton?.remove();
 
           await autofillOverlayContentService.setupOverlayListeners(
             autofillFieldElement,
             autofillFieldData,
             pageDetailsMock,
           );
-          submitButton.dispatchEvent(new KeyboardEvent("keyup", { code: "Enter" }));
+          submitButton?.dispatchEvent(new KeyboardEvent("keyup", { code: "Enter" }));
 
           expect(sendExtensionMessageSpy).not.toHaveBeenCalledWith(
             "formFieldSubmitted",
@@ -1627,7 +1627,7 @@ describe("AutofillOverlayContentService", () => {
             pageDetailsMock,
           );
           await flushPromises();
-          submitButton.dispatchEvent(new KeyboardEvent("keyup", { code: "Enter" }));
+          submitButton?.dispatchEvent(new KeyboardEvent("keyup", { code: "Enter" }));
 
           expect(sendExtensionMessageSpy).toHaveBeenCalledWith(
             "formFieldSubmitted",
@@ -1641,7 +1641,7 @@ describe("AutofillOverlayContentService", () => {
             <div id="shadow-root"></div>
             <button id="button-el">Change Password</button>
           </div>`;
-          const shadowRoot = document.getElementById("shadow-root").attachShadow({ mode: "open" });
+          const shadowRoot = document.getElementById("shadow-root")!.attachShadow({ mode: "open" });
           shadowRoot.innerHTML = `
             <input type="password" id="password-field-1" placeholder="new password" />
           `;
@@ -1668,7 +1668,7 @@ describe("AutofillOverlayContentService", () => {
             pageDetailsMock,
           );
           await flushPromises();
-          buttonElement.dispatchEvent(new KeyboardEvent("keyup", { code: "Enter" }));
+          buttonElement?.dispatchEvent(new KeyboardEvent("keyup", { code: "Enter" }));
 
           expect(sendExtensionMessageSpy).toHaveBeenCalledWith(
             "formFieldSubmitted",
@@ -1716,6 +1716,85 @@ describe("AutofillOverlayContentService", () => {
     });
   });
 
+  describe("refreshMenuLayerPosition", () => {
+    it("calls refreshTopLayerPosition on the inline menu content service", () => {
+      autofillOverlayContentService.refreshMenuLayerPosition();
+
+      expect(inlineMenuContentService.refreshTopLayerPosition).toHaveBeenCalled();
+    });
+
+    it("does not throw if inline menu content service is not available", () => {
+      const serviceWithoutInlineMenu = new AutofillOverlayContentService(
+        domQueryService,
+        domElementVisibilityService,
+        inlineMenuFieldQualificationService,
+      );
+
+      expect(() => serviceWithoutInlineMenu.refreshMenuLayerPosition()).not.toThrow();
+    });
+  });
+
+  describe("getOwnedInlineMenuTagNames", () => {
+    it("returns tag names from the inline menu content service", () => {
+      inlineMenuContentService.getOwnedTagNames.mockReturnValue(["div", "span"]);
+
+      const result = autofillOverlayContentService.getOwnedInlineMenuTagNames();
+
+      expect(result).toEqual(["div", "span"]);
+    });
+
+    it("returns an empty array if inline menu content service is not available", () => {
+      const serviceWithoutInlineMenu = new AutofillOverlayContentService(
+        domQueryService,
+        domElementVisibilityService,
+        inlineMenuFieldQualificationService,
+      );
+
+      const result = serviceWithoutInlineMenu.getOwnedInlineMenuTagNames();
+
+      expect(result).toEqual([]);
+    });
+  });
+
+  describe("getUnownedTopLayerItems", () => {
+    it("returns unowned top layer items from the inline menu content service", () => {
+      const mockElements = document.querySelectorAll("div");
+      inlineMenuContentService.getUnownedTopLayerItems.mockReturnValue(mockElements);
+
+      const result = autofillOverlayContentService.getUnownedTopLayerItems(true);
+
+      expect(result).toEqual(mockElements);
+      expect(inlineMenuContentService.getUnownedTopLayerItems).toHaveBeenCalledWith(true);
+    });
+
+    it("returns undefined if inline menu content service is not available", () => {
+      const serviceWithoutInlineMenu = new AutofillOverlayContentService(
+        domQueryService,
+        domElementVisibilityService,
+        inlineMenuFieldQualificationService,
+      );
+
+      const result = serviceWithoutInlineMenu.getUnownedTopLayerItems();
+
+      expect(result).toBeUndefined();
+    });
+  });
+
+  describe("clearUserFilledFields", () => {
+    it("deletes all user filled fields", () => {
+      const mockElement1 = document.createElement("input") as FillableFormFieldElement;
+      const mockElement2 = document.createElement("input") as FillableFormFieldElement;
+      autofillOverlayContentService["userFilledFields"] = {
+        username: mockElement1,
+        password: mockElement2,
+      };
+
+      autofillOverlayContentService.clearUserFilledFields();
+
+      expect(autofillOverlayContentService["userFilledFields"]).toEqual({});
+    });
+  });
+
   describe("handleOverlayRepositionEvent", () => {
     const repositionEvents = [EVENTS.SCROLL, EVENTS.RESIZE];
     repositionEvents.forEach((repositionEvent) => {
@@ -2049,7 +2128,7 @@ describe("AutofillOverlayContentService", () => {
       });
 
       it("skips focusing an element if no recently focused field exists", async () => {
-        autofillOverlayContentService["mostRecentlyFocusedField"] = undefined;
+        (autofillOverlayContentService as any)["mostRecentlyFocusedField"] = null;
 
         sendMockExtensionMessage({
           command: "redirectAutofillInlineMenuFocusOut",
@@ -2149,7 +2228,6 @@ describe("AutofillOverlayContentService", () => {
       });
 
       it("returns null if the sub frame URL cannot be parsed correctly", async () => {
-        delete globalThis.location;
         globalThis.location = { href: "invalid-base" } as Location;
         sendMockExtensionMessage(
           {

apps/browser/src/autofill/services/inline-menu-field-qualification.service.ts

@@ -945,7 +945,8 @@ export class InlineMenuFieldQualificationService
       !fieldType ||
       !this.usernameFieldTypes.has(fieldType) ||
       this.isExcludedFieldType(field, this.excludedAutofillFieldTypesSet) ||
-      this.fieldHasDisqualifyingAttributeValue(field)
+      this.fieldHasDisqualifyingAttributeValue(field) ||
+      this.isTotpField(field)
     ) {
       return false;
     }