Skip to content

Commit

Permalink
[PM-7980] Inline autofill menu is not shown inside dialog html tag (#…
Browse files Browse the repository at this point in the history 
…11474)

* [PM-7980] Fix inline menu not showing inside dialog html tag

* [PM-7980] Fix inline menu not showing inside dialog html tag

* [PM-7980] Fixing an issue where a dialog element could potentially not represent itself in the #top-layer
  • Loading branch information
@cagonzalezcs
cagonzalezcs authored Oct 15, 2024
1 parent 55ee332 commit 32d12b3
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 53 deletions.
2 changes: 1 addition & 1 deletion apps/browser/src/autofill/background/overlay.background.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
}
}

if (!this.cardAndIdentityCiphers.size) {
if (!this.cardAndIdentityCiphers?.size) {
this.cardAndIdentityCiphers = null;
}

Expand Down
56 changes: 38 additions & 18 deletions ...ser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand All @@ -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(
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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",
Expand All @@ -104,7 +104,7 @@ describe("AutofillInlineMenuContentService", () => {
command: "closeAutofillInlineMenu",
});

expect(unobserveBodyElementSpy).toHaveBeenCalled();
expect(unobserveContainerElementSpy).toHaveBeenCalled();
expect(sendExtensionMessageSpy).toHaveBeenCalledWith("autofillOverlayElementClosed", {
overlayElement: AutofillOverlayElement.Button,
});
Expand All @@ -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", () => {
Expand Down Expand Up @@ -279,7 +279,8 @@ describe("AutofillInlineMenuContentService", () => {
});
});

describe("handleBodyElementMutationObserverUpdate", () => {
describe("handleContainerElementMutationObserverUpdate", () => {
let mockMutationRecord: MockProxy<MutationRecord>;
let buttonElement: HTMLElement;
let listElement: HTMLElement;
let isInlineMenuListVisibleSpy: jest.SpyInstance;
Expand All @@ -289,6 +290,7 @@ describe("AutofillInlineMenuContentService", () => {
<div class="overlay-button"></div>
<div class="overlay-list"></div>
`;
mockMutationRecord = mock<MutationRecord>({ target: globalThis.document.body } as any);
buttonElement = document.querySelector(".overlay-button") as HTMLElement;
listElement = document.querySelector(".overlay-list") as HTMLElement;
autofillInlineMenuContentService["buttonElement"] = buttonElement;
Expand All @@ -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();
Expand All @@ -323,7 +327,9 @@ describe("AutofillInlineMenuContentService", () => {
)
.mockReturnValue(true);

await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"]();
autofillInlineMenuContentService["handleContainerElementMutationObserverUpdate"]([
mockMutationRecord,
]);
await waitForIdleCallback();

expect(globalThis.document.body.insertBefore).not.toHaveBeenCalled();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -409,7 +427,9 @@ describe("AutofillInlineMenuContentService", () => {
1000,
);

await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"]();
autofillInlineMenuContentService["handleContainerElementMutationObserverUpdate"]([
mockMutationRecord,
]);
await waitForIdleCallback();

expect(persistentLastChild.style.getPropertyValue("z-index")).toBe("2147483646");
Expand Down
60 changes: 34 additions & 26 deletions .../browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -102,7 +102,7 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte
return;
}

this.unobserveBodyElement();
this.unobserveContainerElement();
this.closeInlineMenuButton();
this.closeInlineMenuList();
};
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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 && parentDialogElement.open && parentDialogElement.matches(":modal")) {
this.observeContainerElement(parentDialogElement);
parentDialogElement.appendChild(element);
return;
}

this.observeContainerElement(globalThis.document.body);
globalThis.document.body.appendChild(element);
}

Expand Down Expand Up @@ -276,8 +283,8 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte
this.handleInlineMenuElementMutationObserverUpdate,
);

this.bodyElementMutationObserver = new MutationObserver(
this.handleBodyElementMutationObserverUpdate,
this.containerElementMutationObserver = new MutationObserver(
this.handleContainerElementMutationObserverUpdate,
);
};

Expand Down Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -370,27 +375,30 @@ 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()
) {
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;
Expand Down Expand Up @@ -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);
};

/**
Expand Down
37 changes: 29 additions & 8 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 @@ -8,6 +8,7 @@ import {
AutofillOverlayVisibility,
AUTOFILL_OVERLAY_HANDLE_REPOSITION,
AUTOFILL_TRIGGER_FORM_FIELD_SUBMIT,
AUTOFILL_OVERLAY_HANDLE_SCROLL,
} from "@bitwarden/common/autofill/constants";
import { InlineMenuVisibilitySetting } from "@bitwarden/common/autofill/types";
import { CipherType } from "@bitwarden/common/vault/enums";
Expand Down Expand Up @@ -1647,28 +1648,48 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
* the overlay elements on scroll or resize.
*/
private setOverlayRepositionEventListeners() {
const handler = this.useEventHandlersMemo(
const repositionHandler = this.useEventHandlersMemo(
throttle(this.handleOverlayRepositionEvent, 250),
AUTOFILL_OVERLAY_HANDLE_REPOSITION,
);
globalThis.addEventListener(EVENTS.SCROLL, handler, {

const eventTargetDoesNotContainFocusedField = (element: Element) =>
typeof element?.contains === "function" && !element.contains(this.mostRecentlyFocusedField);
const scrollHandler = this.useEventHandlersMemo(
throttle((event) => {
if (eventTargetDoesNotContainFocusedField(event.target as Element)) {
return;
}
repositionHandler(event);
}, 50),
AUTOFILL_OVERLAY_HANDLE_SCROLL,
);

globalThis.addEventListener(EVENTS.SCROLL, scrollHandler, {
capture: true,
passive: true,
});
globalThis.addEventListener(EVENTS.RESIZE, handler);
globalThis.addEventListener(EVENTS.RESIZE, repositionHandler);
}

/**
* Removes the listeners that facilitate repositioning
* the overlay elements on scroll or resize.
*/
private removeOverlayRepositionEventListeners() {
const handler = this.eventHandlersMemo[AUTOFILL_OVERLAY_HANDLE_REPOSITION];
globalThis.removeEventListener(EVENTS.SCROLL, handler, {
capture: true,
});
globalThis.removeEventListener(EVENTS.RESIZE, handler);
globalThis.removeEventListener(
EVENTS.SCROLL,
this.eventHandlersMemo[AUTOFILL_OVERLAY_HANDLE_SCROLL],
{
capture: true,
},
);
globalThis.removeEventListener(
EVENTS.RESIZE,
this.eventHandlersMemo[AUTOFILL_OVERLAY_HANDLE_REPOSITION],
);

delete this.eventHandlersMemo[AUTOFILL_OVERLAY_HANDLE_SCROLL];
delete this.eventHandlersMemo[AUTOFILL_OVERLAY_HANDLE_REPOSITION];
}

Expand Down
Loading

0 comments on commit 32d12b3

Please sign in to comment.