Upstream/bitwarden-browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser.git synced 2024-12-21 16:18:28 +01:00
Code Issues Projects Releases Wiki Activity

[PM-5616] Remove document element mutation observer from Autofill Overlay to fix strange DOM manipulation behavior (#7464)

Browse Source 
* [PM-5616] Remove document element mutation observer from Autofill Overlay to fix strange DOM manipulation behavior

* [PM-5616] Removing unnecessary jest tests
This commit is contained in:
Cesar Gonzalez 2024-01-11 09:54:37 -06:00 committed by GitHub
parent 280cb7e2c0
commit 1b1336d92c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 1 additions and 166 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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() }));
@ -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 = `
<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();
@ -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();

53
apps/browser/src/autofill/services/autofill-overlay-content.service.ts
View File

@ -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