[PM-5670] Autofill not triggering correctly when DOM mutates element with nested form fields (#7518)

apps/browser/src/autofill/jest/autofill-mocks.ts

@@ -7,12 +7,24 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 
 import { OverlayCipherData } from "../background/abstractions/overlay.background";
 import AutofillField from "../models/autofill-field";
+import AutofillForm from "../models/autofill-form";
 import AutofillPageDetails from "../models/autofill-page-details";
 import AutofillScript, { FillScript } from "../models/autofill-script";
 import { InitAutofillOverlayButtonMessage } from "../overlay/abstractions/autofill-overlay-button";
 import { InitAutofillOverlayListMessage } from "../overlay/abstractions/autofill-overlay-list";
 import { GenerateFillScriptOptions, PageDetail } from "../services/abstractions/autofill.service";
 
+function createAutofillFormMock(customFields = {}): AutofillForm {
+  return {
+    opid: "default-form-opid",
+    htmlID: "default-htmlID",
+    htmlAction: "default-htmlAction",
+    htmlMethod: "default-htmlMethod",
+    htmlName: "default-htmlName",
+    ...customFields,
+  };
+}
+
 function createAutofillFieldMock(customFields = {}): AutofillField {
   return {
     opid: "default-input-field-opid",
@@ -258,6 +270,7 @@ function createPortSpyMock(name: string) {
 }
 
 export {
+  createAutofillFormMock,
   createAutofillFieldMock,
   createPageDetailMock,
   createAutofillPageDetailsMock,

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

@@ -1,5 +1,6 @@
 import { mock } from "jest-mock-extended";
 
+import { createAutofillFieldMock, createAutofillFormMock } from "../jest/autofill-mocks";
 import AutofillField from "../models/autofill-field";
 import AutofillForm from "../models/autofill-form";
 import {
@@ -2079,6 +2080,42 @@ describe("CollectAutofillContentService", () => {
       );
     });
 
+    it("removes cached autofill elements that are nested within a removed node", () => {
+      const form = document.createElement("form") as ElementWithOpId<HTMLFormElement>;
+      const usernameInput = document.createElement("input") as ElementWithOpId<FormFieldElement>;
+      usernameInput.setAttribute("type", "text");
+      usernameInput.setAttribute("name", "username");
+      form.appendChild(usernameInput);
+      document.body.appendChild(form);
+      const removedNodes = document.querySelectorAll("form");
+      const autofillForm: AutofillForm = createAutofillFormMock({});
+      const autofillField: AutofillField = createAutofillFieldMock({});
+      collectAutofillContentService["autofillFormElements"] = new Map([[form, autofillForm]]);
+      collectAutofillContentService["autofillFieldElements"] = new Map([
+        [usernameInput, autofillField],
+      ]);
+      collectAutofillContentService["domRecentlyMutated"] = false;
+      collectAutofillContentService["noFieldsFound"] = true;
+      collectAutofillContentService["currentLocationHref"] = window.location.href;
+
+      collectAutofillContentService["handleMutationObserverMutation"]([
+        {
+          type: "childList",
+          addedNodes: null,
+          attributeName: null,
+          attributeNamespace: null,
+          nextSibling: null,
+          oldValue: null,
+          previousSibling: null,
+          removedNodes: removedNodes,
+          target: document.body,
+        },
+      ]);
+
+      expect(collectAutofillContentService["autofillFormElements"].size).toEqual(0);
+      expect(collectAutofillContentService["autofillFieldElements"].size).toEqual(0);
+    });
+
     it("will handle updating the autofill element if any attribute mutations are encountered", () => {
       const mutationRecord: MutationRecord = {
         type: "attributes",
@@ -2389,6 +2426,12 @@ describe("CollectAutofillContentService", () => {
     };
     const updatedAttributes = ["action", "name", "id", "method"];
 
+    beforeEach(() => {
+      collectAutofillContentService["autofillFormElements"] = new Map([
+        [formElement, autofillForm],
+      ]);
+    });
+
     updatedAttributes.forEach((attribute) => {
       it(`will update the ${attribute} value for the form element`, () => {
         jest.spyOn(collectAutofillContentService["autofillFormElements"], "set");
@@ -2454,6 +2497,12 @@ describe("CollectAutofillContentService", () => {
       "data-stripe",
     ];
 
+    beforeEach(() => {
+      collectAutofillContentService["autofillFieldElements"] = new Map([
+        [fieldElement, autofillField],
+      ]);
+    });
+
     updatedAttributes.forEach((attribute) => {
       it(`will update the ${attribute} value for the field element`, async () => {
         jest.spyOn(collectAutofillContentService["autofillFieldElements"], "set");
@@ -2471,26 +2520,6 @@ describe("CollectAutofillContentService", () => {
       });
     });
 
-    it("will check the dom element's visibility if the `style` or `class` attribute has updated ", async () => {
-      jest.spyOn(
-        collectAutofillContentService["domElementVisibilityService"],
-        "isFormFieldViewable",
-      );
-      const attributes = ["class", "style"];
-
-      for (const attribute of attributes) {
-        await collectAutofillContentService["updateAutofillFieldElementData"](
-          attribute,
-          fieldElement,
-          autofillField,
-        );
-
-        expect(
-          collectAutofillContentService["domElementVisibilityService"].isFormFieldViewable,
-        ).toBeCalledWith(fieldElement);
-      }
-    });
-
     it("will not update an attribute value if it is not present in the updateActions object", async () => {
       jest.spyOn(collectAutofillContentService["autofillFieldElements"], "set");
 

apps/browser/src/autofill/services/collect-autofill-content.service.ts

@@ -1029,19 +1029,14 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
         continue;
       }
 
-      if (node instanceof HTMLFormElement || this.isNodeFormFieldElement(node)) {
-        isElementMutated = true;
-        mutatedElements.push(node);
-        continue;
-      }
-
-      const childNodes = this.queryAllTreeWalkerNodes(
+      const autofillElementNodes = this.queryAllTreeWalkerNodes(
         node,
         (node: Node) => node instanceof HTMLFormElement || this.isNodeFormFieldElement(node),
       ) as HTMLElement[];
-      if (childNodes.length) {
+
+      if (autofillElementNodes.length) {
         isElementMutated = true;
-        mutatedElements.push(...childNodes);
+        mutatedElements.push(...autofillElementNodes);
       }
     }
 
@@ -1182,7 +1177,9 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
     }
 
     updateActions[attributeName]();
-    this.autofillFormElements.set(element, dataTarget);
+    if (this.autofillFormElements.has(element)) {
+      this.autofillFormElements.set(element, dataTarget);
+    }
   }
 
   /**
@@ -1233,15 +1230,9 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
 
     updateActions[attributeName]();
 
-    const visibilityAttributesSet = new Set(["class", "style"]);
-    if (
-      visibilityAttributesSet.has(attributeName) &&
-      !dataTarget.htmlClass?.includes("com-bitwarden-browser-animated-fill")
-    ) {
-      dataTarget.viewable = await this.domElementVisibilityService.isFormFieldViewable(element);
+    if (this.autofillFieldElements.has(element)) {
+      this.autofillFieldElements.set(element, dataTarget);
     }
-
-    this.autofillFieldElements.set(element, dataTarget);
   }
 
   /**