Skip to content

Commit

Permalink
[PM-5616] Remove document element mutation observer from Autofill Ove…
Browse files Browse the repository at this point in the history 
…rlay 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
  • Loading branch information
@cagonzalezcs
cagonzalezcs committed Jan 11, 2024
1 parent 280cb7e commit 1b1336d
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 166 deletions.
114 changes: 1 addition & 113 deletions apps/browser/src/autofill/services/autofill-overlay-content.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<MutationObserver>({ observe: jest.fn() }));
Expand All @@ -118,11 +118,6 @@ describe("AutofillOverlayContentService", () => {
autofillOverlayContentService as any,
"handleBodyElementMutationObserverUpdate",
);
const handleDocumentElementMutationObserverUpdateSpy = jest.spyOn(
autofillOverlayContentService as any,
"handleDocumentElementMutationObserverUpdate",
);

autofillOverlayContentService.init();

expect(setupMutationObserverSpy).toHaveBeenCalledTimes(1);
Expand All @@ -134,10 +129,6 @@ describe("AutofillOverlayContentService", () => {
2,
handleBodyElementMutationObserverUpdateSpy,
);
expect(globalThis.MutationObserver).toHaveBeenNthCalledWith(
3,
handleDocumentElementMutationObserverUpdateSpy,
);
});
});

Expand Down Expand Up @@ -1446,105 +1437,6 @@ describe("AutofillOverlayContentService", () => {
});
});

describe("handleDocumentElementMutationObserverUpdate", () => {
let overlayButtonElement: HTMLElement;
let overlayListElement: HTMLElement;

beforeEach(() => {
document.body.innerHTML = `
<div class="overlay-button"></div>
<div class="overlay-list"></div>
`;
document.head.innerHTML = `<title>test</title>`;
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<MutationRecord>(),
]);

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<MutationRecord>(),
]);

expect(globalThis.document.body.appendChild).not.toHaveBeenCalled();
});

it("ignores the mutation record if the record is not of type `childlist`", () => {
autofillOverlayContentService["handleDocumentElementMutationObserverUpdate"]([
mock<MutationRecord>({
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<MutationRecord>({
type: "childList",
addedNodes: mock<NodeList>({ 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();
Expand Down Expand Up @@ -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();
Expand Down
53 changes: 0 additions & 53 deletions apps/browser/src/autofill/services/autofill-overlay-content.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,6 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte
this.overlayButtonElement = globalThis.document.createElement(customElementName);

this.updateCustomElementDefaultStyles(this.overlayButtonElement);
this.moveDocumentElementChildrenToBody(globalThis.document.documentElement.childNodes);
}

/**
Expand Down Expand Up @@ -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,
});
};

/**
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 1b1336d

Please sign in to comment.