[PM-5189] Fixing a weird side issue that appears when a frame within the page triggers a reposition after the inline menu has been built

apps/browser/src/autofill/background/overlay.background.spec.ts

@@ -228,7 +228,7 @@ describe("OverlayBackground", () => {
         await flushPromises();
 
         expect(subFrameOffsetsSpy[tabId]).toStrictEqual(
-          new Map([[1, { left: 4, top: 4, url: "url" }]]),
+          new Map([[1, { left: 4, top: 4, url: "url", parentFrameIds: [1, 0] }]]),
         );
         expect(pageDetailsForTabSpy[tabId].size).toBe(2);
       });
@@ -255,7 +255,7 @@ describe("OverlayBackground", () => {
 
         expect(getFrameDetailsSpy).toHaveBeenCalledTimes(1);
         expect(subFrameOffsetsSpy[tabId]).toStrictEqual(
-          new Map([[1, { left: 0, top: 0, url: "url" }]]),
+          new Map([[1, { left: 0, top: 0, url: "url", parentFrameIds: [] }]]),
         );
       });
 
@@ -1194,7 +1194,7 @@ describe("OverlayBackground", () => {
       });
     });
 
-    describe("checkIsAutofillInlineMenuListVisible", () => {
+    describe("checkIsAutofillInlineMenuListVisible message handler", () => {
       it("sends a message to the top frame of the tab to identify if the inline menu list is visible", () => {
         const sender = mock<chrome.runtime.MessageSender>({ tab: { id: 1 } });
 
@@ -1208,6 +1208,131 @@ describe("OverlayBackground", () => {
       });
     });
 
+    describe("checkShouldRepositionInlineMenu message handler", () => {
+      const tabId = 1;
+      const frameId = 1;
+      const sender = mock<chrome.runtime.MessageSender>({
+        tab: createChromeTabMock({ id: tabId }),
+        frameId,
+      });
+      const otherSender = mock<chrome.runtime.MessageSender>({
+        tab: createChromeTabMock({ id: tabId }),
+        frameId: 2,
+      });
+
+      beforeEach(() => {
+        sendMockExtensionMessage({
+          command: "updateIsFieldCurrentlyFocused",
+          isFieldCurrentlyFocused: true,
+        });
+      });
+
+      it("returns false if the focused field data is not set", async () => {
+        sendMockExtensionMessage(
+          { command: "checkShouldRepositionInlineMenu" },
+          sender,
+          sendResponse,
+        );
+        await flushPromises();
+
+        expect(sendResponse).toHaveBeenCalledWith(false);
+      });
+
+      it("returns false if the sender is from a different tab than the focused field", async () => {
+        const focusedFieldData = createFocusedFieldDataMock();
+        const otherSender = mock<chrome.runtime.MessageSender>({ frameId: 1, tab: { id: 2 } });
+        sendMockExtensionMessage(
+          { command: "updateFocusedFieldData", focusedFieldData },
+          otherSender,
+        );
+
+        sendMockExtensionMessage(
+          { command: "checkShouldRepositionInlineMenu" },
+          sender,
+          sendResponse,
+        );
+        await flushPromises();
+
+        expect(sendResponse).toHaveBeenCalledWith(false);
+      });
+
+      it("returns false if the field is not currently focused", async () => {
+        sendMockExtensionMessage({
+          command: "updateIsFieldCurrentlyFocused",
+          isFieldCurrentlyFocused: false,
+        });
+        const focusedFieldData = createFocusedFieldDataMock();
+        sendMockExtensionMessage({ command: "updateFocusedFieldData", focusedFieldData }, sender);
+
+        sendMockExtensionMessage(
+          { command: "checkShouldRepositionInlineMenu" },
+          sender,
+          sendResponse,
+        );
+        await flushPromises();
+
+        expect(sendResponse).toHaveBeenCalledWith(false);
+      });
+
+      it("returns true if the focused field's frame id is equal to the sender's frame id", async () => {
+        const focusedFieldData = createFocusedFieldDataMock();
+        sendMockExtensionMessage({ command: "updateFocusedFieldData", focusedFieldData }, sender);
+
+        sendMockExtensionMessage(
+          { command: "checkShouldRepositionInlineMenu" },
+          sender,
+          sendResponse,
+        );
+        await flushPromises();
+
+        expect(sendResponse).toHaveBeenCalledWith(true);
+      });
+
+      describe("when the focused field is in a different frame than the sender", () => {
+        it("returns false if the tab does not contain and sub frame offset data", async () => {
+          const focusedFieldData = createFocusedFieldDataMock({ frameId: 2 });
+          sendMockExtensionMessage({ command: "updateFocusedFieldData", focusedFieldData }, sender);
+
+          sendMockExtensionMessage(
+            { command: "checkShouldRepositionInlineMenu" },
+            otherSender,
+            sendResponse,
+          );
+          await flushPromises();
+
+          expect(sendResponse).toHaveBeenCalledWith(false);
+        });
+
+        it("returns true if the sender's frameId is present in any of the parentFrameIds of the tab's sub frames", async () => {
+          const focusedFieldData = createFocusedFieldDataMock();
+          subFrameOffsetsSpy[tabId] = new Map([
+            [frameId, { left: 1, top: 1, url: "https://top-frame.com", parentFrameIds: [2, 0] }],
+          ]);
+          sendMockExtensionMessage({ command: "updateFocusedFieldData", focusedFieldData }, sender);
+
+          sendMockExtensionMessage(
+            { command: "checkShouldRepositionInlineMenu" },
+            otherSender,
+            sendResponse,
+          );
+          await flushPromises();
+
+          expect(sendResponse).toHaveBeenCalledWith(true);
+        });
+      });
+    });
+
+    describe("getCurrentTabFrameId message handler", () => {
+      it("returns the sender's frame id", async () => {
+        const sender = mock<chrome.runtime.MessageSender>({ frameId: 1 });
+
+        sendMockExtensionMessage({ command: "getCurrentTabFrameId" }, sender, sendResponse);
+        await flushPromises();
+
+        expect(sendResponse).toHaveBeenCalledWith(1);
+      });
+    });
+
     describe("unlockCompleted", () => {
       let updateOverlayCiphersSpy: jest.SpyInstance;
 

apps/browser/src/autofill/background/overlay.background.ts

@@ -89,7 +89,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
     checkIsAutofillInlineMenuListVisible: ({ sender }) =>
       this.checkIsAutofillInlineMenuListVisible(sender),
     checkShouldRepositionInlineMenu: ({ sender }) => this.checkShouldRepositionInlineMenu(sender),
-    getCurrentTabFrameId: ({ sender }) => this.getCurrentFrameId(sender),
+    getCurrentTabFrameId: ({ sender }) => this.getSenderFrameId(sender),
     updateSubFrameData: ({ message, sender }) => this.updateSubFrameData(message, sender),
     rebuildSubFrameOffsets: ({ sender }) => this.rebuildSubFrameOffsets(sender),
     collectPageDetailsResponse: ({ message, sender }) => this.storePageDetails(message, sender),
@@ -258,7 +258,14 @@ export class OverlayBackground implements OverlayBackgroundInterface {
     pageDetailsMap.set(sender.frameId, pageDetails);
   }
 
-  private getCurrentFrameId(sender: chrome.runtime.MessageSender) {
+  /**
+   * Returns the frameId, called when calculating sub frame offsets within the tab.
+   * Is used to determine if we should reposition the inline menu when a resize event
+   * occurs within a frame.
+   *
+   * @param sender - The sender of the message
+   */
+  private getSenderFrameId(sender: chrome.runtime.MessageSender) {
     return sender.frameId;
   }
 
@@ -1012,6 +1019,12 @@ export class OverlayBackground implements OverlayBackgroundInterface {
     );
   }
 
+  /**
+   * Handles verifying whether the inline menu should be repositioned. This is used to
+   * guard against removing the inline menu when other frames trigger a resize event.
+   *
+   * @param sender - The sender of the message
+   */
   private checkShouldRepositionInlineMenu(sender: chrome.runtime.MessageSender): boolean {
     if (
       !this.focusedFieldData ||

apps/browser/src/autofill/services/autofill-overlay-content.service.spec.ts

@@ -1078,8 +1078,7 @@ describe("AutofillOverlayContentService", () => {
   });
 
   describe("handleOverlayRepositionEvent", () => {
-    let isInlineMenuButtonVisibleSpy: jest.SpyInstance;
+    let checkShouldRepositionInlineMenuSpy: jest.SpyInstance;
-    let isInlineMenuListVisibleSpy: jest.SpyInstance;
 
     beforeEach(() => {
       document.body.innerHTML = `
@@ -1093,11 +1092,8 @@ describe("AutofillOverlayContentService", () => {
       ) as ElementWithOpId<HTMLInputElement>;
       autofillOverlayContentService["mostRecentlyFocusedField"] = usernameField;
       autofillOverlayContentService["setOverlayRepositionEventListeners"]();
-      isInlineMenuButtonVisibleSpy = jest
+      checkShouldRepositionInlineMenuSpy = jest
-        .spyOn(autofillOverlayContentService as any, "isInlineMenuButtonVisible")
+        .spyOn(autofillOverlayContentService as any, "checkShouldRepositionInlineMenu")
-        .mockResolvedValue(true);
-      isInlineMenuListVisibleSpy = jest
-        .spyOn(autofillOverlayContentService as any, "isInlineMenuListVisible")
         .mockResolvedValue(true);
       jest
         .spyOn(autofillOverlayContentService as any, "recentlyFocusedFieldIsCurrentlyFocused")
@@ -1105,8 +1101,7 @@ describe("AutofillOverlayContentService", () => {
     });
 
     it("skips handling the overlay reposition event if the overlay button and list elements are not visible", async () => {
-      isInlineMenuButtonVisibleSpy.mockResolvedValue(false);
+      checkShouldRepositionInlineMenuSpy.mockResolvedValue(false);
-      isInlineMenuListVisibleSpy.mockResolvedValue(false);
 
       globalThis.dispatchEvent(new Event(EVENTS.RESIZE));
       await flushPromises();
@@ -1124,12 +1119,13 @@ describe("AutofillOverlayContentService", () => {
       });
     });
 
-    it("clears the user interaction timeout", () => {
+    it("clears the user interaction timeout", async () => {
       jest.useFakeTimers();
       const clearTimeoutSpy = jest.spyOn(globalThis, "clearTimeout");
       autofillOverlayContentService["userInteractionEventTimeout"] = setTimeout(jest.fn(), 123);
 
       globalThis.dispatchEvent(new Event(EVENTS.SCROLL));
+      await flushPromises();
 
       expect(clearTimeoutSpy).toHaveBeenCalledWith(expect.anything());
     });

apps/browser/src/autofill/services/autofill-overlay-content.service.ts

@@ -772,7 +772,7 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
    * repositioning of existing overlay elements.
    */
   private handleOverlayRepositionEvent = async () => {
-    if (!(await this.sendExtensionMessage("checkShouldRepositionInlineMenu"))) {
+    if (!(await this.checkShouldRepositionInlineMenu())) {
       return;
     }
 
@@ -1029,6 +1029,10 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
     return (await this.sendExtensionMessage("checkIsInlineMenuCiphersPopulated")) === true;
   }
 
+  private async checkShouldRepositionInlineMenu() {
+    return (await this.sendExtensionMessage("checkShouldRepositionInlineMenu")) === true;
+  }
+
   /**
    * Destroys the autofill overlay content service. This method will
    * disconnect the mutation observers and remove all event listeners.