diff --git a/apps/browser/src/autofill/background/abstractions/overlay.background.ts b/apps/browser/src/autofill/background/abstractions/overlay.background.ts index 1ceeed60a99..c9b230fe18c 100644 --- a/apps/browser/src/autofill/background/abstractions/overlay.background.ts +++ b/apps/browser/src/autofill/background/abstractions/overlay.background.ts @@ -42,6 +42,7 @@ type OverlayPortMessage = { type FocusedFieldData = { focusedFieldStyles: Partial; focusedFieldRects: Partial; + tabId?: number; }; type OverlayCipherData = { @@ -66,14 +67,14 @@ type BackgroundOnMessageHandlerParams = BackgroundMessageParam & BackgroundSende type OverlayBackgroundExtensionMessageHandlers = { [key: string]: CallableFunction; openAutofillOverlay: () => void; - autofillOverlayElementClosed: ({ message }: BackgroundMessageParam) => void; + autofillOverlayElementClosed: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; autofillOverlayAddNewVaultItem: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; getAutofillOverlayVisibility: () => void; checkAutofillOverlayFocused: () => void; focusAutofillOverlayList: () => void; - updateAutofillOverlayPosition: ({ message }: BackgroundMessageParam) => void; + updateAutofillOverlayPosition: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; updateAutofillOverlayHidden: ({ message }: BackgroundMessageParam) => void; - updateFocusedFieldData: ({ message }: BackgroundMessageParam) => void; + updateFocusedFieldData: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; collectPageDetailsResponse: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; unlockCompleted: ({ message }: BackgroundMessageParam) => void; addEditCipherSubmitted: () => void; diff --git a/apps/browser/src/autofill/background/overlay.background.spec.ts b/apps/browser/src/autofill/background/overlay.background.spec.ts index 9f4da8f21f3..05d63b21d19 100644 --- a/apps/browser/src/autofill/background/overlay.background.spec.ts +++ b/apps/browser/src/autofill/background/overlay.background.spec.ts @@ -517,7 +517,7 @@ describe("OverlayBackground", () => { expect(returnValue).toBe(undefined); expect(sendResponse).not.toHaveBeenCalled(); - expect(overlayBackground["overlayElementClosed"]).toHaveBeenCalledWith(message); + expect(overlayBackground["overlayElementClosed"]).toHaveBeenCalledWith(message, sender); }); it("will return a response if the message handler returns a response", async () => { @@ -570,6 +570,26 @@ describe("OverlayBackground", () => { await initOverlayElementPorts(); }); + it("disconnects any expired ports if the sender is not from the same page as the most recently focused field", () => { + const port1 = mock(); + const port2 = mock(); + overlayBackground["expiredPorts"] = [port1, port2]; + const sender = mock({ tab: { id: 1 } }); + const focusedFieldData = createFocusedFieldDataMock({ tabId: 2 }); + sendExtensionRuntimeMessage({ command: "updateFocusedFieldData", focusedFieldData }); + + sendExtensionRuntimeMessage( + { + command: "autofillOverlayElementClosed", + overlayElement: AutofillOverlayElement.Button, + }, + sender, + ); + + expect(port1.disconnect).toHaveBeenCalled(); + expect(port2.disconnect).toHaveBeenCalled(); + }); + it("disconnects the button element port", () => { sendExtensionRuntimeMessage({ command: "autofillOverlayElementClosed", @@ -729,6 +749,23 @@ describe("OverlayBackground", () => { }); }); + it("skips updating the position if the most recently focused field is different than the message sender", () => { + const sender = mock({ tab: { id: 1 } }); + const focusedFieldData = createFocusedFieldDataMock({ tabId: 2 }); + sendExtensionRuntimeMessage({ command: "updateFocusedFieldData", focusedFieldData }); + + sendExtensionRuntimeMessage({ command: "updateAutofillOverlayPosition" }, sender); + + expect(listPortSpy.postMessage).not.toHaveBeenCalledWith({ + command: "updateIframePosition", + styles: expect.anything(), + }); + expect(buttonPortSpy.postMessage).not.toHaveBeenCalledWith({ + command: "updateIframePosition", + styles: expect.anything(), + }); + }); + it("updates the overlay button's position", () => { const focusedFieldData = createFocusedFieldDataMock(); sendExtensionRuntimeMessage({ command: "updateFocusedFieldData", focusedFieldData }); @@ -796,12 +833,14 @@ describe("OverlayBackground", () => { }); it("will post a message to the overlay list facilitating an update of the list's position", () => { + const sender = mock({ tab: { id: 1 } }); const focusedFieldData = createFocusedFieldDataMock(); sendExtensionRuntimeMessage({ command: "updateFocusedFieldData", focusedFieldData }); - overlayBackground["updateOverlayPosition"]({ - overlayElement: AutofillOverlayElement.List, - }); + overlayBackground["updateOverlayPosition"]( + { overlayElement: AutofillOverlayElement.List }, + sender, + ); sendExtensionRuntimeMessage({ command: "updateAutofillOverlayPosition", overlayElement: AutofillOverlayElement.List, @@ -1017,9 +1056,10 @@ describe("OverlayBackground", () => { expect(chrome.runtime.getURL).toHaveBeenCalledWith("overlay/list.css"); expect(overlayBackground["getTranslations"]).toHaveBeenCalled(); expect(overlayBackground["getOverlayCipherData"]).toHaveBeenCalled(); - expect(overlayBackground["updateOverlayPosition"]).toHaveBeenCalledWith({ - overlayElement: AutofillOverlayElement.List, - }); + expect(overlayBackground["updateOverlayPosition"]).toHaveBeenCalledWith( + { overlayElement: AutofillOverlayElement.List }, + listPortSpy.sender, + ); }); it("sets up the overlay button port if the port connection is for the overlay button", async () => { @@ -1032,9 +1072,19 @@ describe("OverlayBackground", () => { expect(overlayBackground["getAuthStatus"]).toHaveBeenCalled(); expect(chrome.runtime.getURL).toHaveBeenCalledWith("overlay/button.css"); expect(overlayBackground["getTranslations"]).toHaveBeenCalled(); - expect(overlayBackground["updateOverlayPosition"]).toHaveBeenCalledWith({ - overlayElement: AutofillOverlayElement.Button, - }); + expect(overlayBackground["updateOverlayPosition"]).toHaveBeenCalledWith( + { overlayElement: AutofillOverlayElement.Button }, + buttonPortSpy.sender, + ); + }); + + it("stores an existing overlay port so that it can be disconnected at a later time", async () => { + overlayBackground["overlayButtonPort"] = mock(); + + await initOverlayElementPorts({ initList: false, initButton: true }); + await flushPromises(); + + expect(overlayBackground["expiredPorts"].length).toBe(1); }); it("gets the system theme", async () => { diff --git a/apps/browser/src/autofill/background/overlay.background.ts b/apps/browser/src/autofill/background/overlay.background.ts index 0e4abcd82d0..56f6a3a2158 100644 --- a/apps/browser/src/autofill/background/overlay.background.ts +++ b/apps/browser/src/autofill/background/overlay.background.ts @@ -54,19 +54,22 @@ class OverlayBackground implements OverlayBackgroundInterface { private userAuthStatus: AuthenticationStatus = AuthenticationStatus.LoggedOut; private overlayButtonPort: chrome.runtime.Port; private overlayListPort: chrome.runtime.Port; + private expiredPorts: chrome.runtime.Port[] = []; private focusedFieldData: FocusedFieldData; private overlayPageTranslations: Record; private iconsServerUrl: string; private readonly extensionMessageHandlers: OverlayBackgroundExtensionMessageHandlers = { openAutofillOverlay: () => this.openOverlay(false), - autofillOverlayElementClosed: ({ message }) => this.overlayElementClosed(message), + autofillOverlayElementClosed: ({ message, sender }) => + this.overlayElementClosed(message, sender), autofillOverlayAddNewVaultItem: ({ message, sender }) => this.addNewVaultItem(message, sender), getAutofillOverlayVisibility: () => this.getOverlayVisibility(), checkAutofillOverlayFocused: () => this.checkOverlayFocused(), focusAutofillOverlayList: () => this.focusOverlayList(), - updateAutofillOverlayPosition: ({ message }) => this.updateOverlayPosition(message), + updateAutofillOverlayPosition: ({ message, sender }) => + this.updateOverlayPosition(message, sender), updateAutofillOverlayHidden: ({ message }) => this.updateOverlayHidden(message), - updateFocusedFieldData: ({ message }) => this.setFocusedFieldData(message), + updateFocusedFieldData: ({ message, sender }) => this.setFocusedFieldData(message, sender), collectPageDetailsResponse: ({ message, sender }) => this.storePageDetails(message, sender), unlockCompleted: ({ message }) => this.unlockCompleted(message), addEditCipherSubmitted: () => this.updateOverlayCiphers(), @@ -302,8 +305,18 @@ class OverlayBackground implements OverlayBackgroundInterface { * the list and button ports and sets them to null. * * @param overlayElement - The overlay element that was closed, either the list or button + * @param sender - The sender of the port message */ - private overlayElementClosed({ overlayElement }: OverlayBackgroundExtensionMessage) { + private overlayElementClosed( + { overlayElement }: OverlayBackgroundExtensionMessage, + sender: chrome.runtime.MessageSender, + ) { + if (sender.tab.id !== this.focusedFieldData?.tabId) { + this.expiredPorts.forEach((port) => port.disconnect()); + this.expiredPorts = []; + return; + } + if (overlayElement === AutofillOverlayElement.Button) { this.overlayButtonPort?.disconnect(); this.overlayButtonPort = null; @@ -320,9 +333,13 @@ class OverlayBackground implements OverlayBackgroundInterface { * is based on the focused field's position and dimensions. * * @param overlayElement - The overlay element to update, either the list or button + * @param sender - The sender of the port message */ - private updateOverlayPosition({ overlayElement }: { overlayElement?: string }) { - if (!overlayElement) { + private updateOverlayPosition( + { overlayElement }: { overlayElement?: string }, + sender: chrome.runtime.MessageSender, + ) { + if (!overlayElement || sender.tab.id !== this.focusedFieldData?.tabId) { return; } @@ -396,9 +413,13 @@ class OverlayBackground implements OverlayBackgroundInterface { * Sets the focused field data to the data passed in the extension message. * * @param focusedFieldData - Contains the rects and styles of the focused field. + * @param sender - The sender of the extension message */ - private setFocusedFieldData({ focusedFieldData }: OverlayBackgroundExtensionMessage) { - this.focusedFieldData = focusedFieldData; + private setFocusedFieldData( + { focusedFieldData }: OverlayBackgroundExtensionMessage, + sender: chrome.runtime.MessageSender, + ) { + this.focusedFieldData = { ...focusedFieldData, tabId: sender.tab.id }; } /** @@ -690,17 +711,11 @@ class OverlayBackground implements OverlayBackgroundInterface { private handlePortOnConnect = async (port: chrome.runtime.Port) => { const isOverlayListPort = port.name === AutofillOverlayPort.List; const isOverlayButtonPort = port.name === AutofillOverlayPort.Button; - if (!isOverlayListPort && !isOverlayButtonPort) { return; } - if (isOverlayListPort) { - this.overlayListPort = port; - } else { - this.overlayButtonPort = port; - } - + this.storeOverlayPort(port); port.onMessage.addListener(this.handleOverlayElementPortMessage); port.postMessage({ command: `initAutofillOverlay${isOverlayListPort ? "List" : "Button"}`, @@ -710,13 +725,47 @@ class OverlayBackground implements OverlayBackgroundInterface { translations: this.getTranslations(), ciphers: isOverlayListPort ? await this.getOverlayCipherData() : null, }); - this.updateOverlayPosition({ - overlayElement: isOverlayListPort - ? AutofillOverlayElement.List - : AutofillOverlayElement.Button, - }); + this.updateOverlayPosition( + { + overlayElement: isOverlayListPort + ? AutofillOverlayElement.List + : AutofillOverlayElement.Button, + }, + port.sender, + ); }; + /** + * Stores the connected overlay port and sets up any existing ports to be disconnected. + * + * @param port - The port to store +| */ + private storeOverlayPort(port: chrome.runtime.Port) { + if (port.name === AutofillOverlayPort.List) { + this.storeExpiredOverlayPort(this.overlayListPort); + this.overlayListPort = port; + return; + } + + if (port.name === AutofillOverlayPort.Button) { + this.storeExpiredOverlayPort(this.overlayButtonPort); + this.overlayButtonPort = port; + } + } + + /** + * When registering a new connection, we want to ensure that the port is disconnected. + * This method places an existing port in the expiredPorts array to be disconnected + * at a later time. + * + * @param port - The port to store in the expiredPorts array + */ + private storeExpiredOverlayPort(port: chrome.runtime.Port | null) { + if (port) { + this.expiredPorts.push(port); + } + } + /** * Handles messages sent to the overlay list or button ports. * 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 4b786e6ca31..43c0817eaf2 100644 --- a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts +++ b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts @@ -186,9 +186,7 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte this.overlayButtonElement.remove(); this.isOverlayButtonVisible = false; - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.sendExtensionMessage("autofillOverlayElementClosed", { + void this.sendExtensionMessage("autofillOverlayElementClosed", { overlayElement: AutofillOverlayElement.Button, }); this.removeOverlayRepositionEventListeners(); @@ -204,9 +202,7 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte this.overlayListElement.remove(); this.isOverlayListVisible = false; - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.sendExtensionMessage("autofillOverlayElementClosed", { + void this.sendExtensionMessage("autofillOverlayElementClosed", { overlayElement: AutofillOverlayElement.List, }); } diff --git a/apps/browser/src/autofill/spec/autofill-mocks.ts b/apps/browser/src/autofill/spec/autofill-mocks.ts index 9c957f6b1be..021b7719b2b 100644 --- a/apps/browser/src/autofill/spec/autofill-mocks.ts +++ b/apps/browser/src/autofill/spec/autofill-mocks.ts @@ -249,6 +249,7 @@ function createFocusedFieldDataMock(customFields = {}) { paddingRight: "6px", paddingLeft: "6px", }, + tabId: 1, ...customFields, }; }