From f1bf290e0bb92d84f755daf3018fafab65341f64 Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Wed, 9 Oct 2024 10:36:09 -0500 Subject: [PATCH] [PM-7980] Fix inline menu not showing inside dialog html tag --- .../autofill/background/overlay.background.ts | 2 +- ...tofill-inline-menu-content.service.spec.ts | 56 +++++++++++------ .../autofill-inline-menu-content.service.ts | 60 +++++++++++-------- 3 files changed, 73 insertions(+), 45 deletions(-) diff --git a/apps/browser/src/autofill/background/overlay.background.ts b/apps/browser/src/autofill/background/overlay.background.ts index b45a4a2548..d8118b5fce 100644 --- a/apps/browser/src/autofill/background/overlay.background.ts +++ b/apps/browser/src/autofill/background/overlay.background.ts @@ -365,7 +365,7 @@ export class OverlayBackground implements OverlayBackgroundInterface { } } - if (!this.cardAndIdentityCiphers.size) { + if (!this.cardAndIdentityCiphers?.size) { this.cardAndIdentityCiphers = null; } diff --git a/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.spec.ts b/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.spec.ts index 8d5e08fc08..c9d86cffc5 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.spec.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.spec.ts @@ -14,7 +14,7 @@ describe("AutofillInlineMenuContentService", () => { let autofillInlineMenuContentService: AutofillInlineMenuContentService; let autofillInit: AutofillInit; let sendExtensionMessageSpy: jest.SpyInstance; - let observeBodyMutationsSpy: jest.SpyInstance; + let observeContainerMutationsSpy: jest.SpyInstance; const waitForIdleCallback = () => new Promise((resolve) => globalThis.requestIdleCallback(resolve)); @@ -25,8 +25,8 @@ describe("AutofillInlineMenuContentService", () => { autofillInlineMenuContentService = new AutofillInlineMenuContentService(); autofillInit = new AutofillInit(domQueryService, null, autofillInlineMenuContentService); autofillInit.init(); - observeBodyMutationsSpy = jest.spyOn( - autofillInlineMenuContentService["bodyElementMutationObserver"] as any, + observeContainerMutationsSpy = jest.spyOn( + autofillInlineMenuContentService["containerElementMutationObserver"] as any, "observe", ); sendExtensionMessageSpy = jest.spyOn( @@ -51,7 +51,7 @@ describe("AutofillInlineMenuContentService", () => { describe("extension message handlers", () => { describe("closeAutofillInlineMenu message handler", () => { beforeEach(() => { - observeBodyMutationsSpy.mockImplementation(); + observeContainerMutationsSpy.mockImplementation(); }); it("closes the inline menu button", async () => { @@ -87,9 +87,9 @@ describe("AutofillInlineMenuContentService", () => { }); it("closes both inline menu elements and removes the body element mutation observer", async () => { - const unobserveBodyElementSpy = jest.spyOn( + const unobserveContainerElementSpy = jest.spyOn( autofillInlineMenuContentService as any, - "unobserveBodyElement", + "unobserveContainerElement", ); sendMockExtensionMessage({ command: "appendAutofillInlineMenuToDom", @@ -104,7 +104,7 @@ describe("AutofillInlineMenuContentService", () => { command: "closeAutofillInlineMenu", }); - expect(unobserveBodyElementSpy).toHaveBeenCalled(); + expect(unobserveContainerElementSpy).toHaveBeenCalled(); expect(sendExtensionMessageSpy).toHaveBeenCalledWith("autofillOverlayElementClosed", { overlayElement: AutofillOverlayElement.Button, }); @@ -127,7 +127,7 @@ describe("AutofillInlineMenuContentService", () => { .spyOn(autofillInlineMenuContentService as any, "isInlineMenuListVisible") .mockResolvedValue(true); jest.spyOn(globalThis.document.body, "appendChild"); - observeBodyMutationsSpy.mockImplementation(); + observeContainerMutationsSpy.mockImplementation(); }); describe("creating the inline menu button", () => { @@ -279,7 +279,8 @@ describe("AutofillInlineMenuContentService", () => { }); }); - describe("handleBodyElementMutationObserverUpdate", () => { + describe("handleContainerElementMutationObserverUpdate", () => { + let mockMutationRecord: MockProxy; let buttonElement: HTMLElement; let listElement: HTMLElement; let isInlineMenuListVisibleSpy: jest.SpyInstance; @@ -289,6 +290,7 @@ describe("AutofillInlineMenuContentService", () => {
`; + mockMutationRecord = mock({ target: globalThis.document.body } as any); buttonElement = document.querySelector(".overlay-button") as HTMLElement; listElement = document.querySelector(".overlay-list") as HTMLElement; autofillInlineMenuContentService["buttonElement"] = buttonElement; @@ -309,7 +311,9 @@ describe("AutofillInlineMenuContentService", () => { autofillInlineMenuContentService["buttonElement"] = undefined; autofillInlineMenuContentService["listElement"] = undefined; - await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"](); + autofillInlineMenuContentService["handleContainerElementMutationObserverUpdate"]([ + mockMutationRecord, + ]); await waitForIdleCallback(); expect(globalThis.document.body.insertBefore).not.toHaveBeenCalled(); @@ -323,7 +327,9 @@ describe("AutofillInlineMenuContentService", () => { ) .mockReturnValue(true); - await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"](); + autofillInlineMenuContentService["handleContainerElementMutationObserverUpdate"]([ + mockMutationRecord, + ]); await waitForIdleCallback(); expect(globalThis.document.body.insertBefore).not.toHaveBeenCalled(); @@ -332,14 +338,18 @@ describe("AutofillInlineMenuContentService", () => { it("skips re-arranging the DOM elements if the last child of the body is non-existent", async () => { document.body.innerHTML = ""; - await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"](); + autofillInlineMenuContentService["handleContainerElementMutationObserverUpdate"]([ + mockMutationRecord, + ]); await waitForIdleCallback(); expect(globalThis.document.body.insertBefore).not.toHaveBeenCalled(); }); it("skips re-arranging the DOM elements if the last child of the body is the overlay list and the second to last child of the body is the overlay button", async () => { - await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"](); + autofillInlineMenuContentService["handleContainerElementMutationObserverUpdate"]([ + mockMutationRecord, + ]); await waitForIdleCallback(); expect(globalThis.document.body.insertBefore).not.toHaveBeenCalled(); @@ -349,7 +359,9 @@ describe("AutofillInlineMenuContentService", () => { listElement.remove(); isInlineMenuListVisibleSpy.mockResolvedValue(false); - await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"](); + autofillInlineMenuContentService["handleContainerElementMutationObserverUpdate"]([ + mockMutationRecord, + ]); await waitForIdleCallback(); expect(globalThis.document.body.insertBefore).not.toHaveBeenCalled(); @@ -359,7 +371,9 @@ describe("AutofillInlineMenuContentService", () => { const injectedElement = document.createElement("div"); document.body.insertBefore(injectedElement, listElement); - await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"](); + autofillInlineMenuContentService["handleContainerElementMutationObserverUpdate"]([ + mockMutationRecord, + ]); await waitForIdleCallback(); expect(globalThis.document.body.insertBefore).toHaveBeenCalledWith( @@ -371,7 +385,9 @@ describe("AutofillInlineMenuContentService", () => { it("positions the overlay button before the overlay list if the elements have inserted in incorrect order", async () => { document.body.appendChild(buttonElement); - await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"](); + autofillInlineMenuContentService["handleContainerElementMutationObserverUpdate"]([ + mockMutationRecord, + ]); await waitForIdleCallback(); expect(globalThis.document.body.insertBefore).toHaveBeenCalledWith( @@ -384,7 +400,9 @@ describe("AutofillInlineMenuContentService", () => { const injectedElement = document.createElement("div"); document.body.appendChild(injectedElement); - await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"](); + autofillInlineMenuContentService["handleContainerElementMutationObserverUpdate"]([ + mockMutationRecord, + ]); await waitForIdleCallback(); expect(globalThis.document.body.insertBefore).toHaveBeenCalledWith( @@ -409,7 +427,9 @@ describe("AutofillInlineMenuContentService", () => { 1000, ); - await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"](); + autofillInlineMenuContentService["handleContainerElementMutationObserverUpdate"]([ + mockMutationRecord, + ]); await waitForIdleCallback(); expect(persistentLastChild.style.getPropertyValue("z-index")).toBe("2147483646"); diff --git a/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.ts b/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.ts index 02d3ae052c..5f9c8f1524 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.ts @@ -30,7 +30,7 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte private buttonElement: HTMLElement; private listElement: HTMLElement; private inlineMenuElementsMutationObserver: MutationObserver; - private bodyElementMutationObserver: MutationObserver; + private containerElementMutationObserver: MutationObserver; private mutationObserverIterations = 0; private mutationObserverIterationsResetTimeout: number | NodeJS.Timeout; private handlePersistentLastChildOverrideTimeout: number | NodeJS.Timeout; @@ -102,7 +102,7 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte return; } - this.unobserveBodyElement(); + this.unobserveContainerElement(); this.closeInlineMenuButton(); this.closeInlineMenuList(); }; @@ -153,7 +153,7 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte } if (!(await this.isInlineMenuButtonVisible())) { - this.appendInlineMenuElementToBody(this.buttonElement); + this.appendInlineMenuElementToDom(this.buttonElement); this.updateInlineMenuElementIsVisibleStatus(AutofillOverlayElement.Button, true); } } @@ -168,7 +168,7 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte } if (!(await this.isInlineMenuListVisible())) { - this.appendInlineMenuElementToBody(this.listElement); + this.appendInlineMenuElementToDom(this.listElement); this.updateInlineMenuElementIsVisibleStatus(AutofillOverlayElement.List, true); } } @@ -196,8 +196,15 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte * * @param element - The inline menu element to append to the body element. */ - private appendInlineMenuElementToBody(element: HTMLElement) { - this.observeBodyElement(); + private appendInlineMenuElementToDom(element: HTMLElement) { + const parentDialogElement = globalThis.document.activeElement?.closest("dialog"); + if (parentDialogElement) { + this.observeContainerElement(parentDialogElement); + parentDialogElement.appendChild(element); + return; + } + + this.observeContainerElement(globalThis.document.body); globalThis.document.body.appendChild(element); } @@ -276,8 +283,8 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte this.handleInlineMenuElementMutationObserverUpdate, ); - this.bodyElementMutationObserver = new MutationObserver( - this.handleBodyElementMutationObserverUpdate, + this.containerElementMutationObserver = new MutationObserver( + this.handleContainerElementMutationObserverUpdate, ); }; @@ -306,19 +313,17 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte } /** - * Sets up a mutation observer for the body element. The mutation observer is used - * to ensure that the inline menu elements are always present at the bottom of the - * body element. + * Sets up a mutation observer for the element which contains the inline menu. */ - private observeBodyElement() { - this.bodyElementMutationObserver?.observe(globalThis.document.body, { childList: true }); + private observeContainerElement(element: HTMLElement) { + this.containerElementMutationObserver?.observe(element, { childList: true }); } /** - * Disconnects the mutation observer for the body element. + * Disconnects the mutation observer for the element which contains the inline menu. */ - private unobserveBodyElement() { - this.bodyElementMutationObserver?.disconnect(); + private unobserveContainerElement() { + this.containerElementMutationObserver?.disconnect(); } /** @@ -370,11 +375,11 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte } /** - * Handles the mutation observer update for the body element. This method will - * ensure that the inline menu elements are always present at the bottom of the - * body element. + * Handles the mutation observer update for the element that contains the inline menu. + * This method will ensure that the inline menu elements are always present at the + * bottom of the container. */ - private handleBodyElementMutationObserverUpdate = () => { + private handleContainerElementMutationObserverUpdate = (mutations: MutationRecord[]) => { if ( (!this.buttonElement && !this.listElement) || this.isTriggeringExcessiveMutationObserverIterations() @@ -382,15 +387,18 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte return; } - requestIdleCallbackPolyfill(this.processBodyElementMutation, { timeout: 500 }); + const containerElement = mutations[0].target as HTMLElement; + requestIdleCallbackPolyfill(() => this.processContainerElementMutation(containerElement), { + timeout: 500, + }); }; /** - * Processes the mutation of the body element. Will trigger when an + * Processes the mutation of the element that contains the inline menu. Will trigger when an * idle moment in the execution of the main thread is detected. */ - private processBodyElementMutation = async () => { - const lastChild = globalThis.document.body.lastElementChild; + private processContainerElementMutation = async (containerElement: HTMLElement) => { + const lastChild = containerElement.lastElementChild; const secondToLastChild = lastChild?.previousElementSibling; const lastChildIsInlineMenuList = lastChild === this.listElement; const lastChildIsInlineMenuButton = lastChild === this.buttonElement; @@ -424,11 +432,11 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte (lastChildIsInlineMenuList && !secondToLastChildIsInlineMenuButton) || (lastChildIsInlineMenuButton && isInlineMenuListVisible) ) { - globalThis.document.body.insertBefore(this.buttonElement, this.listElement); + containerElement.insertBefore(this.buttonElement, this.listElement); return; } - globalThis.document.body.insertBefore(lastChild, this.buttonElement); + containerElement.insertBefore(lastChild, this.buttonElement); }; /**