1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-17 08:43:33 +00:00

[PM-8322] Firefox Inline Autofill Menu Not Propagation Correctly When Switching Tabs

This commit is contained in:
Cesar Gonzalez
2024-05-21 17:22:45 -05:00
parent 983a82c0ba
commit 6fe095351b
6 changed files with 136 additions and 55 deletions

View File

@@ -42,6 +42,7 @@ type OverlayPortMessage = {
type FocusedFieldData = {
focusedFieldStyles: Partial<CSSStyleDeclaration>;
focusedFieldRects: Partial<DOMRect>;
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;

View File

@@ -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<chrome.runtime.Port>();
const port2 = mock<chrome.runtime.Port>();
overlayBackground["expiredPorts"] = [port1, port2];
const sender = mock<chrome.runtime.MessageSender>({ 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<chrome.runtime.MessageSender>({ 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<chrome.runtime.MessageSender>({ 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<chrome.runtime.Port>();
await initOverlayElementPorts({ initList: false, initButton: true });
await flushPromises();
expect(overlayBackground["expiredPorts"].length).toBe(1);
});
it("gets the system theme", async () => {

View File

@@ -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<string, string>;
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.
*

View File

@@ -971,14 +971,6 @@ describe("AutofillOverlayContentService", () => {
expect(document.body.contains(overlayButtonElement)).toEqual(false);
});
it("sends a message to the background indicating that the overlay button has been closed", () => {
autofillOverlayContentService.removeAutofillOverlay();
expect(sendExtensionMessageSpy).toHaveBeenCalledWith("autofillOverlayElementClosed", {
overlayElement: AutofillOverlayElement.Button,
});
});
it("removes the overlay reposition event listeners", () => {
jest.spyOn(globalThis.document.body, "removeEventListener");
jest.spyOn(globalThis, "removeEventListener");
@@ -1020,14 +1012,6 @@ describe("AutofillOverlayContentService", () => {
expect(autofillOverlayContentService["isOverlayListVisible"]).toEqual(false);
expect(document.body.contains(overlayListElement)).toEqual(false);
});
it("sends a message to the extension background indicating that the overlay list has closed", () => {
autofillOverlayContentService.removeAutofillOverlay();
expect(sendExtensionMessageSpy).toHaveBeenCalledWith("autofillOverlayElementClosed", {
overlayElement: AutofillOverlayElement.List,
});
});
});
describe("addNewVaultItem", () => {

View File

@@ -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,
});
}

View File

@@ -249,6 +249,7 @@ function createFocusedFieldDataMock(customFields = {}) {
paddingRight: "6px",
paddingLeft: "6px",
},
tabId: 1,
...customFields,
};
}