From d94d85e20171ec48808ee87cf80900c7d1fdb8bc Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Wed, 12 Jun 2024 16:50:25 -0500 Subject: [PATCH] [PM-5189] Implementing a set threshold for the maximum depth for which we are willing to calculate sub frame offsets --- .../background/overlay.background.spec.ts | 17 ++ .../autofill-overlay-content.service.spec.ts | 214 +++++++++++------- 2 files changed, 145 insertions(+), 86 deletions(-) diff --git a/apps/browser/src/autofill/background/overlay.background.spec.ts b/apps/browser/src/autofill/background/overlay.background.spec.ts index 50ff947fcaf..447abd04425 100644 --- a/apps/browser/src/autofill/background/overlay.background.spec.ts +++ b/apps/browser/src/autofill/background/overlay.background.spec.ts @@ -1310,6 +1310,23 @@ describe("OverlayBackground", () => { }); }); + describe("destroyAutofillInlineMenuListeners", () => { + it("sends a message to the passed frameId that triggers a destruction of the inline menu listeners on that frame", () => { + const sender = mock({ tab: { id: 1 }, frameId: 0 }); + + sendMockExtensionMessage( + { command: "destroyAutofillInlineMenuListeners", subFrameData: { frameId: 10 } }, + sender, + ); + + expect(tabsSendMessageSpy).toHaveBeenCalledWith( + sender.tab, + { command: "destroyAutofillInlineMenuListeners" }, + { frameId: 10 }, + ); + }); + }); + describe("unlockCompleted", () => { let updateInlineMenuCiphersSpy: jest.SpyInstance; 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 cb6ccda9668..d8f40b27bc6 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 @@ -4,7 +4,11 @@ import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authenticatio import { AutofillOverlayVisibility, EVENTS } from "@bitwarden/common/autofill/constants"; import AutofillInit from "../content/autofill-init"; -import { AutofillOverlayElement, RedirectFocusDirection } from "../enums/autofill-overlay.enum"; +import { + AutofillOverlayElement, + MAX_SUB_FRAME_DEPTH, + RedirectFocusDirection, +} from "../enums/autofill-overlay.enum"; import AutofillField from "../models/autofill-field"; import { createAutofillFieldMock } from "../spec/autofill-mocks"; import { flushPromises, sendMockExtensionMessage } from "../spec/testing-utils"; @@ -1034,89 +1038,6 @@ describe("AutofillOverlayContentService", () => { }); }); - describe("destroy", () => { - let autofillFieldElement: ElementWithOpId; - let autofillFieldData: AutofillField; - - beforeEach(() => { - document.body.innerHTML = ` -
- - -
- `; - - autofillFieldElement = document.getElementById( - "username-field", - ) as ElementWithOpId; - autofillFieldElement.opid = "op-1"; - autofillFieldData = createAutofillFieldMock({ - opid: "username-field", - form: "validFormId", - placeholder: "username", - elementNumber: 1, - }); - // 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 - autofillOverlayContentService.setupInlineMenuListenerOnField( - autofillFieldElement, - autofillFieldData, - ); - autofillOverlayContentService["mostRecentlyFocusedField"] = autofillFieldElement; - }); - - it("clears the user interaction event timeout", () => { - jest.spyOn(autofillOverlayContentService as any, "clearUserInteractionEventTimeout"); - - autofillOverlayContentService.destroy(); - - expect(autofillOverlayContentService["clearUserInteractionEventTimeout"]).toHaveBeenCalled(); - }); - - it("de-registers all global event listeners", () => { - jest.spyOn(globalThis.document, "removeEventListener"); - jest.spyOn(globalThis, "removeEventListener"); - jest.spyOn(autofillOverlayContentService as any, "removeOverlayRepositionEventListeners"); - - autofillOverlayContentService.destroy(); - - expect(globalThis.document.removeEventListener).toHaveBeenCalledWith( - EVENTS.VISIBILITYCHANGE, - autofillOverlayContentService["handleVisibilityChangeEvent"], - ); - expect(globalThis.removeEventListener).toHaveBeenCalledWith( - EVENTS.FOCUSOUT, - autofillOverlayContentService["handleFormFieldBlurEvent"], - ); - expect( - autofillOverlayContentService["removeOverlayRepositionEventListeners"], - ).toHaveBeenCalled(); - }); - - it("de-registers any event listeners that are attached to the form field elements", () => { - jest.spyOn(autofillOverlayContentService as any, "removeCachedFormFieldEventListeners"); - jest.spyOn(autofillFieldElement, "removeEventListener"); - jest.spyOn(autofillOverlayContentService["formFieldElements"], "delete"); - - autofillOverlayContentService.destroy(); - - expect( - autofillOverlayContentService["removeCachedFormFieldEventListeners"], - ).toHaveBeenCalledWith(autofillFieldElement); - expect(autofillFieldElement.removeEventListener).toHaveBeenCalledWith( - EVENTS.BLUR, - autofillOverlayContentService["handleFormFieldBlurEvent"], - ); - expect(autofillFieldElement.removeEventListener).toHaveBeenCalledWith( - EVENTS.KEYUP, - autofillOverlayContentService["handleFormFieldKeyupEvent"], - ); - expect(autofillOverlayContentService["formFieldElements"].delete).toHaveBeenCalledWith( - autofillFieldElement, - ); - }); - }); - describe("extension onMessage handlers", () => { describe("openAutofillInlineMenu message handler", () => { let autofillFieldElement: ElementWithOpId; @@ -1563,7 +1484,7 @@ describe("AutofillOverlayContentService", () => { describe("getSubFrameOffsetsFromWindowMessage", () => { it("sends a message to the parent to calculate the sub frame positioning", () => { - jest.spyOn(globalThis.parent, "postMessage"); + jest.spyOn(globalThis.parent, "postMessage").mockImplementation(); const subFrameId = 10; sendMockExtensionMessage({ @@ -1580,6 +1501,7 @@ describe("AutofillOverlayContentService", () => { left: 0, top: 0, parentFrameIds: [], + subFrameDepth: 0, }, }, "*", @@ -1593,6 +1515,29 @@ describe("AutofillOverlayContentService", () => { document.body.innerHTML = ``; }); + it("destroys the inline menu listeners on the origin frame if the depth exceeds the threshold", async () => { + document.body.innerHTML = ``; + const iframe = document.querySelector("iframe") as HTMLIFrameElement; + const subFrameData = { + url: "https://example.com/", + frameId: 10, + left: 0, + top: 0, + parentFrameIds: [1, 2, 3], + subFrameDepth: MAX_SUB_FRAME_DEPTH, + }; + const event = mock(); + // @ts-expect-error - Need to mock the source to be the iframe content window + event.source = iframe.contentWindow; + event.data.subFrameData = subFrameData; + sendExtensionMessageSpy.mockResolvedValue(4); + + await autofillOverlayContentService["calculateSubFramePositioning"](event); + await flushPromises(); + + expect(globalThis.parent.postMessage).not.toHaveBeenCalled(); + }); + it("calculates the sub frame offset for the current frame and sends those values to the parent if not in the top frame", async () => { Object.defineProperty(window, "top", { value: null, @@ -1606,6 +1551,7 @@ describe("AutofillOverlayContentService", () => { left: 0, top: 0, parentFrameIds: [1, 2, 3], + subFrameDepth: 0, }; const event = mock(); // @ts-expect-error - Need to mock the source to be the iframe content window @@ -1625,6 +1571,7 @@ describe("AutofillOverlayContentService", () => { parentFrameIds: [1, 2, 3, 4], top: 2, url: "https://example.com/", + subFrameDepth: 1, }, }, "*", @@ -1640,6 +1587,7 @@ describe("AutofillOverlayContentService", () => { left: 0, top: 0, parentFrameIds: [1, 2, 3], + subFrameDepth: 1, }; const event = mock(); // @ts-expect-error - Need to mock the source to be the iframe content window @@ -1656,13 +1604,14 @@ describe("AutofillOverlayContentService", () => { top: 2, url: "https://example.com/", parentFrameIds: [1, 2, 3], + subFrameDepth: 2, }, }); }); }); }); - describe("checkMostRecentlyFocusedFieldHasValue", () => { + describe("checkMostRecentlyFocusedFieldHasValue message handler", () => { it("returns true if the most recently focused field has a truthy value", async () => { autofillOverlayContentService["mostRecentlyFocusedField"] = mock< ElementWithOpId @@ -1680,5 +1629,98 @@ describe("AutofillOverlayContentService", () => { expect(sendResponseSpy).toHaveBeenCalledWith(true); }); }); + + describe("destroyAutofillInlineMenuListeners message handler", () => { + it("destroys the inline menu listeners", () => { + jest.spyOn(autofillOverlayContentService, "destroy"); + + sendMockExtensionMessage({ command: "destroyAutofillInlineMenuListeners" }); + + expect(autofillOverlayContentService.destroy).toHaveBeenCalled(); + }); + }); + }); + + describe("destroy", () => { + let autofillFieldElement: ElementWithOpId; + let autofillFieldData: AutofillField; + + beforeEach(() => { + document.body.innerHTML = ` +
+ + +
+ `; + + autofillFieldElement = document.getElementById( + "username-field", + ) as ElementWithOpId; + autofillFieldElement.opid = "op-1"; + autofillFieldData = createAutofillFieldMock({ + opid: "username-field", + form: "validFormId", + placeholder: "username", + elementNumber: 1, + }); + // 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 + autofillOverlayContentService.setupInlineMenuListenerOnField( + autofillFieldElement, + autofillFieldData, + ); + autofillOverlayContentService["mostRecentlyFocusedField"] = autofillFieldElement; + }); + + it("clears the user interaction event timeout", () => { + jest.spyOn(autofillOverlayContentService as any, "clearUserInteractionEventTimeout"); + + autofillOverlayContentService.destroy(); + + expect(autofillOverlayContentService["clearUserInteractionEventTimeout"]).toHaveBeenCalled(); + }); + + it("de-registers all global event listeners", () => { + jest.spyOn(globalThis.document, "removeEventListener"); + jest.spyOn(globalThis, "removeEventListener"); + jest.spyOn(autofillOverlayContentService as any, "removeOverlayRepositionEventListeners"); + + autofillOverlayContentService.destroy(); + + expect(globalThis.document.removeEventListener).toHaveBeenCalledWith( + EVENTS.VISIBILITYCHANGE, + autofillOverlayContentService["handleVisibilityChangeEvent"], + ); + expect(globalThis.removeEventListener).toHaveBeenCalledWith( + EVENTS.FOCUSOUT, + autofillOverlayContentService["handleFormFieldBlurEvent"], + ); + expect( + autofillOverlayContentService["removeOverlayRepositionEventListeners"], + ).toHaveBeenCalled(); + }); + + it("de-registers any event listeners that are attached to the form field elements", () => { + jest.spyOn(autofillOverlayContentService as any, "removeCachedFormFieldEventListeners"); + jest.spyOn(autofillFieldElement, "removeEventListener"); + jest.spyOn(autofillOverlayContentService["formFieldElements"], "delete"); + + autofillOverlayContentService.destroy(); + + expect( + autofillOverlayContentService["removeCachedFormFieldEventListeners"], + ).toHaveBeenCalledWith(autofillFieldElement); + expect(autofillFieldElement.removeEventListener).toHaveBeenCalledWith( + EVENTS.BLUR, + autofillOverlayContentService["handleFormFieldBlurEvent"], + ); + expect(autofillFieldElement.removeEventListener).toHaveBeenCalledWith( + EVENTS.KEYUP, + autofillOverlayContentService["handleFormFieldKeyupEvent"], + ); + expect(autofillOverlayContentService["formFieldElements"].delete).toHaveBeenCalledWith( + autofillFieldElement, + ); + }); }); });