From da18e36b88a17e498b91d5cc6f6814eefa9fe6f9 Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Fri, 3 May 2024 02:57:08 -0500 Subject: [PATCH] [PM-5189] Refactoring implementation --- .../abstractions/overlay.background.ts | 12 +++--- .../background/overlay.background.spec.ts | 39 ++++++++++--------- .../autofill/background/overlay.background.ts | 20 +++++----- .../autofill-overlay-iframe.service.ts | 6 +-- .../pages/list/autofill-overlay-list.spec.ts | 8 ++-- .../pages/list/autofill-overlay-list.ts | 4 +- .../shared/autofill-overlay-page-element.ts | 4 +- .../autofill-overlay-content.service.ts | 2 +- .../autofill-overlay-content.service.spec.ts | 22 +++++++---- .../autofill-overlay-content.service.ts | 4 +- 10 files changed, 67 insertions(+), 54 deletions(-) diff --git a/apps/browser/src/autofill/background/abstractions/overlay.background.ts b/apps/browser/src/autofill/background/abstractions/overlay.background.ts index f4877a61883..7cfd5ab8b21 100644 --- a/apps/browser/src/autofill/background/abstractions/overlay.background.ts +++ b/apps/browser/src/autofill/background/abstractions/overlay.background.ts @@ -132,22 +132,22 @@ type OverlayButtonPortMessageHandlers = { [key: string]: CallableFunction; autofillInlineMenuButtonClicked: ({ port }: PortConnectionParam) => void; closeAutofillInlineMenu: ({ port }: PortConnectionParam) => void; - forceCloseAutofillOverlay: ({ port }: PortConnectionParam) => void; - overlayPageBlurred: () => void; - redirectInlineMenuFocusOut: ({ message, port }: PortOnMessageHandlerParams) => void; + forceCloseAutofillInlineMenu: ({ port }: PortConnectionParam) => void; + autofillInlineMenuBlurred: () => void; + redirectAutofillInlineMenuFocusOut: ({ message, port }: PortOnMessageHandlerParams) => void; updateOverlayPageColorScheme: () => void; }; type OverlayListPortMessageHandlers = { [key: string]: CallableFunction; checkAutofillOverlayButtonFocused: () => void; - forceCloseAutofillOverlay: ({ port }: PortConnectionParam) => void; - overlayPageBlurred: () => void; + forceCloseAutofillInlineMenu: ({ port }: PortConnectionParam) => void; + autofillInlineMenuBlurred: () => void; unlockVault: ({ port }: PortConnectionParam) => void; fillSelectedListItem: ({ message, port }: PortOnMessageHandlerParams) => void; addNewVaultItem: ({ port }: PortConnectionParam) => void; viewSelectedCipher: ({ message, port }: PortOnMessageHandlerParams) => void; - redirectInlineMenuFocusOut: ({ message, port }: PortOnMessageHandlerParams) => void; + redirectAutofillInlineMenuFocusOut: ({ message, port }: PortOnMessageHandlerParams) => void; updateAutofillOverlayListHeight: ({ message, port }: PortOnMessageHandlerParams) => void; }; diff --git a/apps/browser/src/autofill/background/overlay.background.spec.ts b/apps/browser/src/autofill/background/overlay.background.spec.ts index d4785444b20..d1f1df05d13 100644 --- a/apps/browser/src/autofill/background/overlay.background.spec.ts +++ b/apps/browser/src/autofill/background/overlay.background.spec.ts @@ -1160,7 +1160,7 @@ describe("OverlayBackground", () => { expect(overlayBackground["openInlineMenu"]).toHaveBeenCalled(); }); - // TODO: The tests for `closeAutofillInlineMenu` and `forceCloseAutofillOverlay` need to be fleshed out + // TODO: The tests for `closeAutofillInlineMenu` and `forceCloseAutofillInlineMenu` need to be fleshed out describe("closeAutofillInlineMenu", () => { it("sends a `closeOverlay` message to the sender tab", () => { jest.spyOn(BrowserApi, "tabSendMessage"); @@ -1178,12 +1178,12 @@ describe("OverlayBackground", () => { }); }); - describe("forceCloseAutofillOverlay", () => { + describe("forceCloseAutofillInlineMenu", () => { it("sends a `closeOverlay` message to the sender tab with a `forceCloseAutofillInlineMenu` flag of `true` set", () => { jest.spyOn(BrowserApi, "tabSendMessage"); sendPortMessage(buttonMessageConnectorPortSpy, { - command: "forceCloseAutofillOverlay", + command: "forceCloseAutofillInlineMenu", portKey, }); @@ -1195,27 +1195,27 @@ describe("OverlayBackground", () => { }); }); - describe("overlayPageBlurred", () => { + describe("autofillInlineMenuBlurred", () => { it("checks if the overlay list is focused", () => { - jest.spyOn(overlayBackground as any, "checkOverlayListFocused"); + jest.spyOn(overlayBackground as any, "checkInlineMenuListFocused"); sendPortMessage(buttonMessageConnectorPortSpy, { - command: "overlayPageBlurred", + command: "autofillInlineMenuBlurred", portKey, }); - expect(overlayBackground["checkOverlayListFocused"]).toHaveBeenCalled(); + expect(overlayBackground["checkInlineMenuListFocused"]).toHaveBeenCalled(); }); }); - describe("redirectInlineMenuFocusOut", () => { + describe("redirectAutofillInlineMenuFocusOut", () => { beforeEach(() => { jest.spyOn(BrowserApi, "tabSendMessageData"); }); it("ignores the redirect message if the direction is not provided", () => { sendPortMessage(buttonMessageConnectorPortSpy, { - command: "redirectInlineMenuFocusOut", + command: "redirectAutofillInlineMenuFocusOut", portKey, }); @@ -1224,14 +1224,14 @@ describe("OverlayBackground", () => { it("sends the redirect message if the direction is provided", () => { sendPortMessage(buttonMessageConnectorPortSpy, { - command: "redirectInlineMenuFocusOut", + command: "redirectAutofillInlineMenuFocusOut", direction: RedirectFocusDirection.Next, portKey, }); expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( buttonMessageConnectorPortSpy.sender.tab, - "redirectInlineMenuFocusOut", + "redirectAutofillInlineMenuFocusOut", { direction: RedirectFocusDirection.Next }, ); }); @@ -1258,12 +1258,12 @@ describe("OverlayBackground", () => { }); }); - describe("forceCloseAutofillOverlay", () => { + describe("forceCloseAutofillInlineMenu", () => { it("sends a `closeOverlay` message to the sender tab with a `forceCloseAutofillInlineMenu` flag of `true` set", () => { jest.spyOn(BrowserApi, "tabSendMessage"); sendPortMessage(listMessageConnectorPortSpy, { - command: "forceCloseAutofillOverlay", + command: "forceCloseAutofillInlineMenu", portKey, }); @@ -1275,11 +1275,14 @@ describe("OverlayBackground", () => { }); }); - describe("overlayPageBlurred", () => { + describe("autofillInlineMenuBlurred", () => { it("checks on the focus state of the overlay button", () => { jest.spyOn(overlayBackground as any, "checkOverlayButtonFocused").mockImplementation(); - sendPortMessage(listMessageConnectorPortSpy, { command: "overlayPageBlurred", portKey }); + sendPortMessage(listMessageConnectorPortSpy, { + command: "autofillInlineMenuBlurred", + portKey, + }); expect(overlayBackground["checkOverlayButtonFocused"]).toHaveBeenCalled(); }); @@ -1521,16 +1524,16 @@ describe("OverlayBackground", () => { }); }); - describe("redirectInlineMenuFocusOut", () => { + describe("redirectAutofillInlineMenuFocusOut", () => { it("redirects focus out of the overlay list", async () => { const message = { - command: "redirectInlineMenuFocusOut", + command: "redirectAutofillInlineMenuFocusOut", direction: RedirectFocusDirection.Next, portKey, }; const redirectOverlayFocusOutSpy = jest.spyOn( overlayBackground as any, - "redirectInlineMenuFocusOut", + "redirectAutofillInlineMenuFocusOut", ); sendPortMessage(listMessageConnectorPortSpy, message); diff --git a/apps/browser/src/autofill/background/overlay.background.ts b/apps/browser/src/autofill/background/overlay.background.ts index 7f32245be3e..c8e6b2ed3bd 100644 --- a/apps/browser/src/autofill/background/overlay.background.ts +++ b/apps/browser/src/autofill/background/overlay.background.ts @@ -96,23 +96,23 @@ class OverlayBackground implements OverlayBackgroundInterface { private readonly overlayButtonPortMessageHandlers: OverlayButtonPortMessageHandlers = { autofillInlineMenuButtonClicked: ({ port }) => this.handleInlineMenuButtonClicked(port), closeAutofillInlineMenu: ({ port }) => this.closeInlineMenu(port.sender), - forceCloseAutofillOverlay: ({ port }) => + forceCloseAutofillInlineMenu: ({ port }) => this.closeInlineMenu(port.sender, { forceCloseAutofillInlineMenu: true }), - overlayPageBlurred: () => this.checkOverlayListFocused(), - redirectInlineMenuFocusOut: ({ message, port }) => + autofillInlineMenuBlurred: () => this.checkInlineMenuListFocused(), + redirectAutofillInlineMenuFocusOut: ({ message, port }) => this.redirectInlineMenuFocusOut(message, port), updateOverlayPageColorScheme: () => this.updateButtonPageColorScheme(), }; private readonly overlayListPortMessageHandlers: OverlayListPortMessageHandlers = { checkAutofillOverlayButtonFocused: () => this.checkOverlayButtonFocused(), - forceCloseAutofillOverlay: ({ port }) => + forceCloseAutofillInlineMenu: ({ port }) => this.closeInlineMenu(port.sender, { forceCloseAutofillInlineMenu: true }), - overlayPageBlurred: () => this.checkOverlayButtonFocused(), + autofillInlineMenuBlurred: () => this.checkOverlayButtonFocused(), unlockVault: ({ port }) => this.unlockVault(port), fillSelectedListItem: ({ message, port }) => this.fillSelectedOverlayListItem(message, port), addNewVaultItem: ({ port }) => this.getNewVaultItemDetails(port), viewSelectedCipher: ({ message, port }) => this.viewSelectedCipher(message, port), - redirectInlineMenuFocusOut: ({ message, port }) => + redirectAutofillInlineMenuFocusOut: ({ message, port }) => this.redirectInlineMenuFocusOut(message, port), updateAutofillOverlayListHeight: ({ message }) => this.updateOverlayListHeight(message), }; @@ -388,7 +388,7 @@ class OverlayBackground implements OverlayBackgroundInterface { */ private checkInlineMenuFocused() { if (this.overlayListPort) { - this.checkOverlayListFocused(); + this.checkInlineMenuListFocused(); return; } @@ -406,7 +406,7 @@ class OverlayBackground implements OverlayBackgroundInterface { /** * Posts a message to the overlay list iframe to check if it is focused. */ - private checkOverlayListFocused() { + private checkInlineMenuListFocused() { this.overlayListPort?.postMessage({ command: "checkAutofillOverlayListFocused" }); } @@ -798,7 +798,9 @@ class OverlayBackground implements OverlayBackgroundInterface { return; } - void BrowserApi.tabSendMessageData(sender.tab, "redirectInlineMenuFocusOut", { direction }); + void BrowserApi.tabSendMessageData(sender.tab, "redirectAutofillInlineMenuFocusOut", { + direction, + }); } /** diff --git a/apps/browser/src/autofill/overlay/iframe-content/autofill-overlay-iframe.service.ts b/apps/browser/src/autofill/overlay/iframe-content/autofill-overlay-iframe.service.ts index 6516522b48f..b998741bda3 100644 --- a/apps/browser/src/autofill/overlay/iframe-content/autofill-overlay-iframe.service.ts +++ b/apps/browser/src/autofill/overlay/iframe-content/autofill-overlay-iframe.service.ts @@ -323,7 +323,7 @@ class AutofillOverlayIframeService implements AutofillOverlayIframeServiceInterf * Triggers a forced closure of the autofill overlay. This is used when the * mutation observer is triggered excessively. */ - private forceCloseAutofillOverlay() { + private forceCloseAutofillInlineMenu() { void this.sendExtensionMessage("closeAutofillInlineMenu", { forceClose: true }); } @@ -342,7 +342,7 @@ class AutofillOverlayIframeService implements AutofillOverlayIframeServiceInterf } if (this.foreignMutationsCount >= 10) { - this.forceCloseAutofillOverlay(); + this.forceCloseAutofillInlineMenu(); break; } @@ -397,7 +397,7 @@ class AutofillOverlayIframeService implements AutofillOverlayIframeServiceInterf if (this.mutationObserverIterations > 20) { clearTimeout(this.mutationObserverIterationsResetTimeout); resetCounters(); - this.forceCloseAutofillOverlay(); + this.forceCloseAutofillInlineMenu(); return true; } diff --git a/apps/browser/src/autofill/overlay/pages/list/autofill-overlay-list.spec.ts b/apps/browser/src/autofill/overlay/pages/list/autofill-overlay-list.spec.ts index 5c8c3fbda7d..98647caa24e 100644 --- a/apps/browser/src/autofill/overlay/pages/list/autofill-overlay-list.spec.ts +++ b/apps/browser/src/autofill/overlay/pages/list/autofill-overlay-list.spec.ts @@ -384,7 +384,7 @@ describe("AutofillOverlayList", () => { globalThis.dispatchEvent(new Event("blur")); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "overlayPageBlurred", portKey }, + { command: "autofillInlineMenuBlurred", portKey }, "*", ); }); @@ -407,7 +407,7 @@ describe("AutofillOverlayList", () => { ); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "redirectInlineMenuFocusOut", direction: "previous", portKey }, + { command: "redirectAutofillInlineMenuFocusOut", direction: "previous", portKey }, "*", ); }); @@ -416,7 +416,7 @@ describe("AutofillOverlayList", () => { globalThis.document.dispatchEvent(new KeyboardEvent("keydown", { code: "Tab" })); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "redirectInlineMenuFocusOut", direction: "next", portKey }, + { command: "redirectAutofillInlineMenuFocusOut", direction: "next", portKey }, "*", ); }); @@ -425,7 +425,7 @@ describe("AutofillOverlayList", () => { globalThis.document.dispatchEvent(new KeyboardEvent("keydown", { code: "Escape" })); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "redirectInlineMenuFocusOut", direction: "current", portKey }, + { command: "redirectAutofillInlineMenuFocusOut", direction: "current", portKey }, "*", ); }); diff --git a/apps/browser/src/autofill/overlay/pages/list/autofill-overlay-list.ts b/apps/browser/src/autofill/overlay/pages/list/autofill-overlay-list.ts index f71b4912acb..ba28bab732c 100644 --- a/apps/browser/src/autofill/overlay/pages/list/autofill-overlay-list.ts +++ b/apps/browser/src/autofill/overlay/pages/list/autofill-overlay-list.ts @@ -24,7 +24,7 @@ class AutofillOverlayList extends AutofillOverlayPageElement { private readonly showCiphersPerPage = 6; private readonly overlayListWindowMessageHandlers: OverlayListWindowMessageHandlers = { initAutofillOverlayList: ({ message }) => this.initAutofillOverlayList(message), - checkAutofillOverlayListFocused: () => this.checkOverlayListFocused(), + checkAutofillOverlayListFocused: () => this.checkInlineMenuListFocused(), updateOverlayListCiphers: ({ message }) => this.updateListItems(message.ciphers), focusInlineMenuList: () => this.focusInlineMenuList(), }; @@ -482,7 +482,7 @@ class AutofillOverlayList extends AutofillOverlayPageElement { * Validates whether the overlay list iframe is currently focused. * If not focused, will check if the button element is focused. */ - private checkOverlayListFocused() { + private checkInlineMenuListFocused() { if (globalThis.document.hasFocus()) { return; } diff --git a/apps/browser/src/autofill/overlay/pages/shared/autofill-overlay-page-element.ts b/apps/browser/src/autofill/overlay/pages/shared/autofill-overlay-page-element.ts index 0e69c7c1067..9011ffa241d 100644 --- a/apps/browser/src/autofill/overlay/pages/shared/autofill-overlay-page-element.ts +++ b/apps/browser/src/autofill/overlay/pages/shared/autofill-overlay-page-element.ts @@ -113,7 +113,7 @@ class AutofillOverlayPageElement extends HTMLElement { * Handles the window blur event. */ private handleWindowBlurEvent = () => { - this.postMessageToParent({ command: "overlayPageBlurred" }); + this.postMessageToParent({ command: "autofillInlineMenuBlurred" }); }; /** @@ -150,7 +150,7 @@ class AutofillOverlayPageElement extends HTMLElement { * @param direction - The direction to redirect the focus out */ private redirectOverlayFocusOutMessage(direction: string) { - this.postMessageToParent({ command: "redirectInlineMenuFocusOut", direction }); + this.postMessageToParent({ command: "redirectAutofillInlineMenuFocusOut", direction }); } } diff --git a/apps/browser/src/autofill/services/abstractions/autofill-overlay-content.service.ts b/apps/browser/src/autofill/services/abstractions/autofill-overlay-content.service.ts index 34166948da4..0276c98d9ac 100644 --- a/apps/browser/src/autofill/services/abstractions/autofill-overlay-content.service.ts +++ b/apps/browser/src/autofill/services/abstractions/autofill-overlay-content.service.ts @@ -18,7 +18,7 @@ export type AutofillOverlayContentExtensionMessageHandlers = { blurMostRecentOverlayField: () => void; bgUnlockPopoutOpened: () => void; bgVaultItemRepromptPopoutOpened: () => void; - redirectInlineMenuFocusOut: ({ message }: AutofillExtensionMessageParam) => void; + redirectAutofillInlineMenuFocusOut: ({ message }: AutofillExtensionMessageParam) => void; updateAutofillInlineMenuVisibility: ({ message }: AutofillExtensionMessageParam) => void; getSubFrameOffsets: ({ message }: AutofillExtensionMessageParam) => Promise; getSubFrameOffsetsFromWindowMessage: ({ message }: AutofillExtensionMessageParam) => void; 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 ffe963dc688..c85baa6f0d1 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 @@ -1003,7 +1003,9 @@ describe("AutofillOverlayContentService", () => { it("skips focusing an element if the overlay is not visible", async () => { isInlineMenuListVisibleSpy.mockResolvedValue(false); - await autofillOverlayContentService.redirectInlineMenuFocusOut(RedirectFocusDirection.Next); + await autofillOverlayContentService["redirectInlineMenuFocusOut"]( + RedirectFocusDirection.Next, + ); expect(findTabsSpy).not.toHaveBeenCalled(); }); @@ -1011,13 +1013,15 @@ describe("AutofillOverlayContentService", () => { it("skips focusing an element if no recently focused field exists", async () => { autofillOverlayContentService["mostRecentlyFocusedField"] = undefined; - await autofillOverlayContentService.redirectInlineMenuFocusOut(RedirectFocusDirection.Next); + await autofillOverlayContentService["redirectInlineMenuFocusOut"]( + RedirectFocusDirection.Next, + ); expect(findTabsSpy).not.toHaveBeenCalled(); }); it("focuses the most recently focused field if the focus direction is `Current`", async () => { - await autofillOverlayContentService.redirectInlineMenuFocusOut( + await autofillOverlayContentService["redirectInlineMenuFocusOut"]( RedirectFocusDirection.Current, ); @@ -1027,7 +1031,7 @@ describe("AutofillOverlayContentService", () => { it("removes the overlay if the focus direction is `Current`", async () => { jest.useFakeTimers(); - await autofillOverlayContentService.redirectInlineMenuFocusOut( + await autofillOverlayContentService["redirectInlineMenuFocusOut"]( RedirectFocusDirection.Current, ); jest.advanceTimersByTime(150); @@ -1043,7 +1047,9 @@ describe("AutofillOverlayContentService", () => { nextFocusableElement, ]); - await autofillOverlayContentService.redirectInlineMenuFocusOut(RedirectFocusDirection.Next); + await autofillOverlayContentService["redirectInlineMenuFocusOut"]( + RedirectFocusDirection.Next, + ); expect(findTabsSpy).toHaveBeenCalledWith(globalThis.document.body, { getShadowRoot: true }); }); @@ -1051,7 +1057,7 @@ describe("AutofillOverlayContentService", () => { it("focuses the previous focusable element if the focus direction is `Previous`", async () => { jest.spyOn(previousFocusableElement, "focus"); - await autofillOverlayContentService.redirectInlineMenuFocusOut( + await autofillOverlayContentService["redirectInlineMenuFocusOut"]( RedirectFocusDirection.Previous, ); @@ -1062,7 +1068,9 @@ describe("AutofillOverlayContentService", () => { it("focuses the next focusable element if the focus direction is `Next`", async () => { jest.spyOn(nextFocusableElement, "focus"); - await autofillOverlayContentService.redirectInlineMenuFocusOut(RedirectFocusDirection.Next); + await autofillOverlayContentService["redirectInlineMenuFocusOut"]( + RedirectFocusDirection.Next, + ); expect(autofillFieldFocusSpy).not.toHaveBeenCalled(); expect(nextFocusableElement.focus).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 cafed02dd97..b3e2467973b 100644 --- a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts +++ b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts @@ -46,7 +46,7 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte blurMostRecentOverlayField: () => this.blurMostRecentOverlayField(), bgUnlockPopoutOpened: () => this.blurMostRecentOverlayField(true), bgVaultItemRepromptPopoutOpened: () => this.blurMostRecentOverlayField(true), - redirectInlineMenuFocusOut: ({ message }) => + redirectAutofillInlineMenuFocusOut: ({ message }) => this.redirectInlineMenuFocusOut(message?.data?.direction), updateAutofillInlineMenuVisibility: ({ message }) => this.updateAutofillInlineMenuVisibility(message), @@ -188,7 +188,7 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte * * @param direction - The direction to redirect the focus out. */ - async redirectInlineMenuFocusOut(direction?: string) { + private async redirectInlineMenuFocusOut(direction?: string) { if (!direction || !this.mostRecentlyFocusedField || !(await this.isInlineMenuListVisible())) { return; }