[PM-5189] Reworking how we handle updating the inline menu position

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

@@ -52,14 +52,17 @@ export type CloseInlineMenuMessage = {
   overlayElement?: string;
 };
 
+export type ToggleInlineMenuHiddenMessage = {
+  isInlineMenuHidden?: boolean;
+  setTransparentInlineMenu?: boolean;
+};
+
 export type OverlayBackgroundExtensionMessage = {
   command: string;
   portKey?: string;
   tab?: chrome.tabs.Tab;
   sender?: string;
   details?: AutofillPageDetails;
-  isInlineMenuHidden?: boolean;
-  setTransparentInlineMenu?: boolean;
   isFieldCurrentlyFocused?: boolean;
   isFieldCurrentlyFilling?: boolean;
   subFrameData?: SubFrameOffsetData;
@@ -67,7 +70,8 @@ export type OverlayBackgroundExtensionMessage = {
   styles?: Partial<CSSStyleDeclaration>;
   data?: LockedVaultPendingNotificationsData;
 } & OverlayAddNewItemMessage &
-  CloseInlineMenuMessage;
+  CloseInlineMenuMessage &
+  ToggleInlineMenuHiddenMessage;
 
 export type OverlayPortMessage = {
   [key: string]: any;
@@ -99,6 +103,7 @@ export type OverlayBackgroundExtensionMessageHandlers = {
   [key: string]: CallableFunction;
   autofillOverlayElementClosed: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
   autofillOverlayAddNewVaultItem: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
+  triggerAutofillOverlayReposition: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
   checkIsInlineMenuCiphersPopulated: ({ sender }: BackgroundSenderParam) => void;
   updateFocusedFieldData: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
   updateIsFieldCurrentlyFocused: ({ message }: BackgroundMessageParam) => void;
@@ -117,11 +122,9 @@ export type OverlayBackgroundExtensionMessageHandlers = {
   toggleAutofillInlineMenuHidden: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
   checkIsAutofillInlineMenuButtonVisible: ({ sender }: BackgroundSenderParam) => void;
   checkIsAutofillInlineMenuListVisible: ({ sender }: BackgroundSenderParam) => void;
-  checkShouldRepositionInlineMenu: ({ sender }: BackgroundSenderParam) => boolean;
   getCurrentTabFrameId: ({ sender }: BackgroundSenderParam) => number;
   updateSubFrameData: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
-  rebuildSubFrameOffsets: ({ sender }: BackgroundSenderParam) => void;
+  triggerSubFrameFocusInRebuild: ({ sender }: BackgroundSenderParam) => void;
-  repositionAutofillInlineMenuForSubFrame: ({ sender }: BackgroundSenderParam) => void;
   destroyAutofillInlineMenuListeners: ({
     message,
     sender,

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

@@ -396,7 +396,7 @@ describe("OverlayBackground", () => {
         jest.useFakeTimers();
         overlayBackground["delayedUpdateInlineMenuPositionTimeout"] = setTimeout(jest.fn, 650);
         const sender = mock<chrome.runtime.MessageSender>({ tab, frameId: middleFrameId });
-        jest.spyOn(overlayBackground as any, "updateInlineMenuPositionAfterSubFrameRebuild");
+        jest.spyOn(overlayBackground as any, "updateInlineMenuPositionAfterRepositionEvent");
 
         sendMockExtensionMessage(
           {
@@ -409,12 +409,12 @@ describe("OverlayBackground", () => {
         jest.advanceTimersByTime(650);
 
         expect(
-          overlayBackground["updateInlineMenuPositionAfterSubFrameRebuild"],
+          overlayBackground["updateInlineMenuPositionAfterRepositionEvent"],
         ).toHaveBeenCalled();
       });
     });
 
-    describe("updateInlineMenuPositionAfterSubFrameRebuild", () => {
+    describe("updateInlineMenuPositionAfterRepositionEvent", () => {
       let sender: chrome.runtime.MessageSender;
 
       async function flushInlineMenuUpdatePromises() {

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

@@ -1,4 +1,5 @@
-import { firstValueFrom } from "rxjs";
+import { firstValueFrom, Subject } from "rxjs";
+import { debounceTime, switchMap } from "rxjs/operators";
 
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
@@ -48,6 +49,7 @@ import {
   SubFrameOffsetData,
   SubFrameOffsetsForTab,
   CloseInlineMenuMessage,
+  ToggleInlineMenuHiddenMessage,
 } from "./abstractions/overlay.background";
 
 export class OverlayBackground implements OverlayBackgroundInterface {
@@ -65,6 +67,8 @@ export class OverlayBackground implements OverlayBackgroundInterface {
   private inlineMenuFadeInTimeout: number | NodeJS.Timeout;
   private delayedUpdateInlineMenuPositionTimeout: number | NodeJS.Timeout;
   private delayedCloseTimeout: number | NodeJS.Timeout;
+  private repositionInlineMenu$ = new Subject<chrome.runtime.MessageSender>();
+  private rebuildSubFrameOffsets$ = new Subject<chrome.runtime.MessageSender>();
   private focusedFieldData: FocusedFieldData;
   private isFieldCurrentlyFocused: boolean = false;
   private isFieldCurrentlyFilling: boolean = false;
@@ -73,6 +77,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
     autofillOverlayElementClosed: ({ message, sender }) =>
       this.overlayElementClosed(message, sender),
     autofillOverlayAddNewVaultItem: ({ message, sender }) => this.addNewVaultItem(message, sender),
+    triggerAutofillOverlayReposition: ({ sender }) => this.triggerOverlayReposition(sender),
     checkIsInlineMenuCiphersPopulated: ({ sender }) =>
       this.checkIsInlineMenuCiphersPopulated(sender),
     updateFocusedFieldData: ({ message, sender }) => this.setFocusedFieldData(message, sender),
@@ -88,16 +93,13 @@ export class OverlayBackground implements OverlayBackgroundInterface {
     updateAutofillInlineMenuPosition: ({ message, sender }) =>
       this.updateInlineMenuPosition(message, sender),
     toggleAutofillInlineMenuHidden: ({ message, sender }) =>
-      this.updateInlineMenuHidden(message, sender),
+      this.toggleInlineMenuHidden(message, sender),
     checkIsAutofillInlineMenuButtonVisible: ({ sender }) =>
       this.checkIsInlineMenuButtonVisible(sender),
     checkIsAutofillInlineMenuListVisible: ({ sender }) => this.checkIsInlineMenuListVisible(sender),
-    checkShouldRepositionInlineMenu: ({ sender }) => this.checkShouldRepositionInlineMenu(sender),
     getCurrentTabFrameId: ({ sender }) => this.getSenderFrameId(sender),
     updateSubFrameData: ({ message, sender }) => this.updateSubFrameData(message, sender),
-    rebuildSubFrameOffsets: ({ sender }) => this.rebuildSubFrameOffsets(sender),
+    triggerSubFrameFocusInRebuild: ({ sender }) => this.triggerSubFrameFocusInRebuild(sender),
-    repositionAutofillInlineMenuForSubFrame: ({ sender }) =>
-      this.repositionInlineMenuForSubFrame(sender),
     destroyAutofillInlineMenuListeners: ({ message, sender }) =>
       this.triggerDestroyInlineMenuListeners(sender.tab, message.subFrameData.frameId),
     collectPageDetailsResponse: ({ message, sender }) => this.storePageDetails(message, sender),
@@ -138,7 +140,20 @@ export class OverlayBackground implements OverlayBackgroundInterface {
     private i18nService: I18nService,
     private platformUtilsService: PlatformUtilsService,
     private themeStateService: ThemeStateService,
-  ) {}
+  ) {
+    this.repositionInlineMenu$
+      .pipe(
+        debounceTime(500),
+        switchMap((sender) => this.repositionInlineMenu(sender)),
+      )
+      .subscribe();
+    this.rebuildSubFrameOffsets$
+      .pipe(
+        debounceTime(200),
+        switchMap((sender) => this.rebuildSubFrameOffsets(sender)),
+      )
+      .subscribe();
+  }
 
   /**
    * Sets up the extension message listeners and gets the settings for the
@@ -411,23 +426,6 @@ export class OverlayBackground implements OverlayBackgroundInterface {
     }
   }
 
-  /**
-   * Handles rebuilding the sub frame offsets when the tab is repositioned or scrolled.
-   * Will trigger a re-positioning of the inline menu list and button. Note that we
-   * do not trigger an update to sub frame data if the sender is the frame that has
-   * the field currently focused. We trigger a re-calculation of the field's position
-   * and as a result, the sub frame offsets of that frame will be updated.
-   *
-   * @param sender - The sender of the message
-   */
-  private async repositionInlineMenuForSubFrame(sender: chrome.runtime.MessageSender) {
-    if (sender.frameId === this.focusedFieldData?.frameId) {
-      return;
-    }
-
-    await this.rebuildSubFrameOffsets(sender);
-  }
-
   /**
    * Triggers a delayed repositioning of the inline menu. Used in cases where the page in some way
    * is resized, scrolled, or when a sub frame is interacted with.
@@ -438,7 +436,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
     this.clearDelayedUpdateInlineMenuPositionTimeout();
     this.delayedUpdateInlineMenuPositionTimeout = globalThis.setTimeout(async () => {
       this.clearDelayedUpdateInlineMenuPositionTimeout();
-      await this.updateInlineMenuPositionAfterSubFrameRebuild(sender);
+      await this.updateInlineMenuPositionAfterRepositionEvent(sender);
     }, 650);
   }
 
@@ -449,7 +447,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
    *
    * @param sender - The sender of the message
    */
-  private async updateInlineMenuPositionAfterSubFrameRebuild(sender: chrome.runtime.MessageSender) {
+  private async updateInlineMenuPositionAfterRepositionEvent(sender: chrome.runtime.MessageSender) {
     if (!this.isFieldCurrentlyFocused) {
       return;
     }
@@ -791,8 +789,8 @@ export class OverlayBackground implements OverlayBackgroundInterface {
    * @param display - The display property of the inline menu, either "block" or "none"
    * @param sender - The sender of the extension message
    */
-  private async updateInlineMenuHidden(
+  private async toggleInlineMenuHidden(
-    { isInlineMenuHidden, setTransparentInlineMenu }: OverlayBackgroundExtensionMessage,
+    { isInlineMenuHidden, setTransparentInlineMenu }: ToggleInlineMenuHiddenMessage,
     sender: chrome.runtime.MessageSender,
   ) {
     this.clearInlineMenuFadeInTimeout();
@@ -1122,13 +1120,15 @@ export class OverlayBackground implements OverlayBackgroundInterface {
    *
    * @param sender - The sender of the message
    */
-  private checkShouldRepositionInlineMenu(sender: chrome.runtime.MessageSender): boolean {
+  private async checkShouldRepositionInlineMenu(
+    sender: chrome.runtime.MessageSender,
+  ): Promise<boolean> {
     if (!this.focusedFieldData || sender.tab.id !== this.focusedFieldData.tabId) {
       return false;
     }
 
     if (this.focusedFieldData.frameId === sender.frameId) {
-      return true;
+      return await this.checkIsInlineMenuButtonVisible(sender);
     }
 
     const subFrameOffsetsForTab = this.subFrameOffsetsForTab[sender.tab.id];
@@ -1347,4 +1347,47 @@ export class OverlayBackground implements OverlayBackgroundInterface {
       this.inlineMenuButtonPort = null;
     }
   };
+
+  private async triggerOverlayReposition(sender: chrome.runtime.MessageSender) {
+    if (await this.checkShouldRepositionInlineMenu(sender)) {
+      await this.toggleInlineMenuHidden({ isInlineMenuHidden: true }, sender);
+      this.repositionInlineMenu$.next(sender);
+    }
+  }
+
+  private repositionInlineMenu = async (sender: chrome.runtime.MessageSender) => {
+    if (!this.isFieldCurrentlyFocused) {
+      await this.closeInlineMenuAfterReposition(sender);
+      return;
+    }
+
+    const isFieldWithinViewport = await BrowserApi.tabSendMessage(
+      sender.tab,
+      { command: "checkIsMostRecentlyFocusedFieldWithinViewport" },
+      { frameId: this.focusedFieldData.frameId },
+    );
+    if (!isFieldWithinViewport) {
+      await this.closeInlineMenuAfterReposition(sender);
+      return;
+    }
+
+    if (this.focusedFieldData.frameId > 0 && sender.frameId !== this.focusedFieldData.frameId) {
+      this.rebuildSubFrameOffsets$.next(sender);
+      return;
+    }
+
+    await this.updateInlineMenuPositionAfterRepositionEvent(sender);
+  };
+
+  private async closeInlineMenuAfterReposition(sender: chrome.runtime.MessageSender) {
+    await this.toggleInlineMenuHidden(
+      { isInlineMenuHidden: false, setTransparentInlineMenu: true },
+      sender,
+    );
+    this.closeInlineMenu(sender, { forceCloseInlineMenu: true });
+  }
+
+  private async triggerSubFrameFocusInRebuild(sender: chrome.runtime.MessageSender) {
+    this.rebuildSubFrameOffsets$.next(sender);
+  }
 }

apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.ts

@@ -211,7 +211,10 @@ export class AutofillInlineMenuList extends AutofillInlineMenuPageElement {
     if (this.cipherListScrollDebounceTimeout) {
       clearTimeout(this.cipherListScrollDebounceTimeout);
     }
-    this.cipherListScrollDebounceTimeout = setTimeout(this.handleDebouncedScrollEvent, 300);
+    this.cipherListScrollDebounceTimeout = globalThis.setTimeout(
+      this.handleDebouncedScrollEvent,
+      300,
+    );
   };
 
   /**

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

@@ -22,6 +22,7 @@ export type AutofillOverlayContentExtensionMessageHandlers = {
   addNewVaultItemFromOverlay: () => void;
   blurMostRecentlyFocusedField: () => void;
   unsetMostRecentlyFocusedField: () => void;
+  checkIsMostRecentlyFocusedFieldWithinViewport: () => Promise<boolean>;
   bgUnlockPopoutOpened: () => void;
   bgVaultItemRepromptPopoutOpened: () => void;
   redirectAutofillInlineMenuFocusOut: ({ message }: AutofillExtensionMessageParam) => void;

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

@@ -64,6 +64,8 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
     addNewVaultItemFromOverlay: () => this.addNewVaultItem(),
     blurMostRecentlyFocusedField: () => this.blurMostRecentlyFocusedField(),
     unsetMostRecentlyFocusedField: () => this.unsetMostRecentlyFocusedField(),
+    checkIsMostRecentlyFocusedFieldWithinViewport: () =>
+      this.checkIsMostRecentlyFocusedFieldWithinViewport(),
     bgUnlockPopoutOpened: () => this.blurMostRecentlyFocusedField(true),
     bgVaultItemRepromptPopoutOpened: () => this.blurMostRecentlyFocusedField(true),
     redirectAutofillInlineMenuFocusOut: ({ message }) =>
@@ -615,7 +617,7 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
       focusedFieldRects: { width, height, top, left },
     };
 
-    void this.sendExtensionMessage("updateFocusedFieldData", {
+    await this.sendExtensionMessage("updateFocusedFieldData", {
       focusedFieldData: this.focusedFieldData,
     });
   }
@@ -1024,7 +1026,7 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
     globalThis.addEventListener(
       EVENTS.SCROLL,
       this.useEventHandlersMemo(
-        throttle(this.handleOverlayRepositionEvent, 150),
+        throttle(this.handleOverlayRepositionEvent, 200),
         AUTOFILL_OVERLAY_ON_SCROLL,
       ),
       {
@@ -1034,7 +1036,7 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
     globalThis.addEventListener(
       EVENTS.RESIZE,
       this.useEventHandlersMemo(
-        throttle(this.handleOverlayRepositionEvent, 150),
+        throttle(this.handleOverlayRepositionEvent, 200),
         AUTOFILL_OVERLAY_ON_RESIZE,
       ),
     );
@@ -1066,68 +1068,7 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
    * repositioning of existing overlay elements.
    */
   private handleOverlayRepositionEvent = async () => {
-    if (!(await this.checkShouldRepositionInlineMenu())) {
+    await this.sendExtensionMessage("triggerAutofillOverlayReposition");
-      return;
-    }
-
-    this.repositionInlineMenuForSubFrame();
-    this.toggleInlineMenuHidden(true);
-    this.clearUserInteractionEventTimeout();
-    this.userInteractionEventTimeout = globalThis.setTimeout(
-      this.triggerOverlayRepositionUpdates,
-      750,
-    );
-  };
-
-  /**
-   * Triggers a rebuild of a sub frame's offsets within the tab.
-   */
-  private repositionInlineMenuForSubFrame() {
-    this.clearRecalculateSubFrameOffsetsTimeout();
-    this.recalculateSubFrameOffsetsTimeout = globalThis.setTimeout(
-      () => void this.sendExtensionMessage("repositionAutofillInlineMenuForSubFrame"),
-      150,
-    );
-  }
-
-  /**
-   * Triggers the overlay reposition updates. This method ensures that the overlay elements
-   * are correctly positioned when the viewport scrolls or repositions.
-   */
-  private triggerOverlayRepositionUpdates = async () => {
-    if (!this.recentlyFocusedFieldIsCurrentlyFocused()) {
-      this.toggleInlineMenuHidden(false, true);
-      void this.sendExtensionMessage("closeAutofillInlineMenu", {
-        forceCloseInlineMenu: true,
-      });
-      return;
-    }
-
-    await this.updateMostRecentlyFocusedField(this.mostRecentlyFocusedField);
-    this.updateInlineMenuElementsPosition();
-    this.clearCloseInlineMenuOnFilledFieldTimeout();
-    this.closeInlineMenuOnFilledFieldTimeout = globalThis.setTimeout(async () => {
-      this.toggleInlineMenuHidden(false, true);
-      if (
-        await this.hideInlineMenuListOnFilledField(
-          this.mostRecentlyFocusedField as FillableFormFieldElement,
-        )
-      ) {
-        void this.sendExtensionMessage("closeAutofillInlineMenu", {
-          overlayElement: AutofillOverlayElement.List,
-          forceCloseInlineMenu: true,
-        });
-      }
-    }, 50);
-    this.clearUserInteractionEventTimeout();
-
-    if (this.isFocusedFieldWithinViewportBounds()) {
-      return;
-    }
-
-    void this.sendExtensionMessage("closeAutofillInlineMenu", {
-      forceCloseInlineMenu: true,
-    });
   };
 
   private setupRebuildSubFrameOffsetsListeners = () => {
@@ -1140,7 +1081,7 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
   };
 
   private handleSubFrameFocusInEvent = () => {
-    this.rebuildSubFrameOffsets();
+    void this.sendExtensionMessage("triggerSubFrameFocusInRebuild");
 
     globalThis.removeEventListener(EVENTS.FOCUS, this.handleSubFrameFocusInEvent);
     globalThis.document.body.removeEventListener(
@@ -1154,11 +1095,11 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
     );
   };
 
-  private rebuildSubFrameOffsets = () => {
+  private async checkIsMostRecentlyFocusedFieldWithinViewport() {
-    this.clearUserInteractionEventTimeout();
+    await this.updateMostRecentlyFocusedField(this.mostRecentlyFocusedField);
-    this.clearRecalculateSubFrameOffsetsTimeout();
+
-    void this.sendExtensionMessage("rebuildSubFrameOffsets");
+    return this.isFocusedFieldWithinViewportBounds();
-  };
+  }
 
   /**
    * Checks if the focused field is present within the bounds of the viewport.