From 1b1336d92cf0e28a2bb894d22e5a7d9157f56447 Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Thu, 11 Jan 2024 09:54:37 -0600 Subject: [PATCH] [PM-5616] Remove document element mutation observer from Autofill Overlay to fix strange DOM manipulation behavior (#7464) * [PM-5616] Remove document element mutation observer from Autofill Overlay to fix strange DOM manipulation behavior * [PM-5616] Removing unnecessary jest tests --- .../autofill-overlay-content.service.spec.ts | 114 +----------------- .../autofill-overlay-content.service.ts | 53 -------- 2 files changed, 1 insertion(+), 166 deletions(-) diff --git a/apps/browser/src/autofill/services/autofill-overlay-content.service.spec.ts b/apps/browser/src/autofill/services/autofill-overlay-content.service.spec.ts index f3aa77258f..01d63ca8f2 100644 --- a/apps/browser/src/autofill/services/autofill-overlay-content.service.spec.ts +++ b/apps/browser/src/autofill/services/autofill-overlay-content.service.spec.ts @@ -106,7 +106,7 @@ describe("AutofillOverlayContentService", () => { expect(window.addEventListener).toHaveBeenCalledWith("focusout", handleFormFieldBlurEventSpy); }); - it("sets up mutation observers for the body and html element", () => { + it("sets up mutation observers for the body element", () => { jest .spyOn(globalThis, "MutationObserver") .mockImplementation(() => mock({ observe: jest.fn() })); @@ -118,11 +118,6 @@ describe("AutofillOverlayContentService", () => { autofillOverlayContentService as any, "handleBodyElementMutationObserverUpdate", ); - const handleDocumentElementMutationObserverUpdateSpy = jest.spyOn( - autofillOverlayContentService as any, - "handleDocumentElementMutationObserverUpdate", - ); - autofillOverlayContentService.init(); expect(setupMutationObserverSpy).toHaveBeenCalledTimes(1); @@ -134,10 +129,6 @@ describe("AutofillOverlayContentService", () => { 2, handleBodyElementMutationObserverUpdateSpy, ); - expect(globalThis.MutationObserver).toHaveBeenNthCalledWith( - 3, - handleDocumentElementMutationObserverUpdateSpy, - ); }); }); @@ -1446,105 +1437,6 @@ describe("AutofillOverlayContentService", () => { }); }); - describe("handleDocumentElementMutationObserverUpdate", () => { - let overlayButtonElement: HTMLElement; - let overlayListElement: HTMLElement; - - beforeEach(() => { - document.body.innerHTML = ` -
-
- `; - document.head.innerHTML = `test`; - overlayButtonElement = document.querySelector(".overlay-button") as HTMLElement; - overlayListElement = document.querySelector(".overlay-list") as HTMLElement; - autofillOverlayContentService["overlayButtonElement"] = overlayButtonElement; - autofillOverlayContentService["overlayListElement"] = overlayListElement; - autofillOverlayContentService["isOverlayListVisible"] = true; - jest.spyOn(globalThis.document.body, "appendChild"); - jest - .spyOn( - autofillOverlayContentService as any, - "isTriggeringExcessiveMutationObserverIterations", - ) - .mockReturnValue(false); - }); - - it("skips modification of the DOM if the overlay button and list elements are not present", () => { - autofillOverlayContentService["overlayButtonElement"] = undefined; - autofillOverlayContentService["overlayListElement"] = undefined; - - autofillOverlayContentService["handleDocumentElementMutationObserverUpdate"]([ - mock(), - ]); - - expect(globalThis.document.body.appendChild).not.toHaveBeenCalled(); - }); - - it("skips modification of the DOM if excessive mutation events are being triggered", () => { - jest - .spyOn( - autofillOverlayContentService as any, - "isTriggeringExcessiveMutationObserverIterations", - ) - .mockReturnValue(true); - - autofillOverlayContentService["handleDocumentElementMutationObserverUpdate"]([ - mock(), - ]); - - expect(globalThis.document.body.appendChild).not.toHaveBeenCalled(); - }); - - it("ignores the mutation record if the record is not of type `childlist`", () => { - autofillOverlayContentService["handleDocumentElementMutationObserverUpdate"]([ - mock({ - type: "attributes", - }), - ]); - - expect(globalThis.document.body.appendChild).not.toHaveBeenCalled(); - }); - - it("ignores the mutation record if the record does not contain any added nodes", () => { - autofillOverlayContentService["handleDocumentElementMutationObserverUpdate"]([ - mock({ - type: "childList", - addedNodes: mock({ length: 0 }), - }), - ]); - - expect(globalThis.document.body.appendChild).not.toHaveBeenCalled(); - }); - - it("ignores mutations for the document body and head", () => { - autofillOverlayContentService["handleDocumentElementMutationObserverUpdate"]([ - { - type: "childList", - addedNodes: document.querySelectorAll("body, head"), - } as unknown as MutationRecord, - ]); - - expect(globalThis.document.body.appendChild).not.toHaveBeenCalled(); - }); - - it("appends the identified node to the body", async () => { - jest.useFakeTimers(); - const injectedElement = document.createElement("div"); - injectedElement.id = "test"; - document.documentElement.appendChild(injectedElement); - autofillOverlayContentService["handleDocumentElementMutationObserverUpdate"]([ - { - type: "childList", - addedNodes: document.querySelectorAll("#test"), - } as unknown as MutationRecord, - ]); - jest.advanceTimersByTime(10); - - expect(globalThis.document.body.appendChild).toHaveBeenCalledWith(injectedElement); - }); - }); - describe("isTriggeringExcessiveMutationObserverIterations", () => { it("clears any existing reset timeout", () => { jest.useFakeTimers(); @@ -1642,13 +1534,9 @@ describe("AutofillOverlayContentService", () => { it("disconnects all mutation observers", () => { autofillOverlayContentService["setupMutationObserver"](); jest.spyOn(autofillOverlayContentService["bodyElementMutationObserver"], "disconnect"); - jest.spyOn(autofillOverlayContentService["documentElementMutationObserver"], "disconnect"); autofillOverlayContentService.destroy(); - expect( - autofillOverlayContentService["documentElementMutationObserver"].disconnect, - ).toHaveBeenCalled(); expect( autofillOverlayContentService["bodyElementMutationObserver"].disconnect, ).toHaveBeenCalled(); 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 c713c6ea41..4234e0cf4f 100644 --- a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts +++ b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts @@ -747,7 +747,6 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte this.overlayButtonElement = globalThis.document.createElement(customElementName); this.updateCustomElementDefaultStyles(this.overlayButtonElement); - this.moveDocumentElementChildrenToBody(globalThis.document.documentElement.childNodes); } /** @@ -900,13 +899,6 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte this.bodyElementMutationObserver = new MutationObserver( this.handleBodyElementMutationObserverUpdate, ); - - this.documentElementMutationObserver = new MutationObserver( - this.handleDocumentElementMutationObserverUpdate, - ); - this.documentElementMutationObserver.observe(globalThis.document.documentElement, { - childList: true, - }); }; /** @@ -1034,51 +1026,6 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte globalThis.document.body.insertBefore(lastChild, this.overlayButtonElement); }; - /** - * Handles the mutation observer update for the document element. This - * method will ensure that any elements added to the document element - * are appended to the body element. - * - * @param mutationRecords - The mutation records that triggered the update. - */ - private handleDocumentElementMutationObserverUpdate = (mutationRecords: MutationRecord[]) => { - if ( - (!this.overlayButtonElement && !this.overlayListElement) || - this.isTriggeringExcessiveMutationObserverIterations() - ) { - return; - } - - for (const record of mutationRecords) { - if (record.type !== "childList" || record.addedNodes.length === 0) { - continue; - } - - this.moveDocumentElementChildrenToBody(record.addedNodes); - } - }; - - /** - * Moves the passed nodes to the body element. This method is used to ensure that - * any elements added to the document element are higher in the DOM than the overlay - * elements. - * - * @param nodes - The nodes to move to the body element. - */ - private moveDocumentElementChildrenToBody(nodes: NodeList) { - const ignoredElements = new Set([globalThis.document.body, globalThis.document.head]); - for (const node of nodes) { - if (ignoredElements.has(node as HTMLElement)) { - continue; - } - - // This is a workaround for an issue where the document element's children - // are not appended to the body element. This forces the children to be - // appended on the next tick of the event loop. - setTimeout(() => globalThis.document.body.appendChild(node), 0); - } - } - /** * Identifies if the mutation observer is triggering excessive iterations. * Will trigger a blur of the most recently focused field and remove the