[PM-14054] Fixing scroll-based repositioning of the inline menu on Sh…

…adowDOM elements (#11803) * [PM-14054] Fixing scroll-based repositioning of inline menu when inline menu is focused * [PM-14054] Fixing scroll-based repositioning of the inline menu on ShadowDOM elements * [PM-14054] Fixing scroll-based repositioning of the inline menu on ShadowDOM elements * [PM-14054] Fixing scroll-based repositioning of the inline menu on ShadowDOM elements
bitwarden · Nov 1, 2024 · a4c6731 · a4c6731
1 parent 227e9c4
commit a4c6731
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 39 deletions.
diff --git a/apps/browser/src/autofill/background/abstractions/overlay.background.ts b/apps/browser/src/autofill/background/abstractions/overlay.background.ts
@@ -216,7 +216,6 @@ export type OverlayBackgroundExtensionMessageHandlers = {
   getCurrentTabFrameId: ({ sender }: BackgroundSenderParam) => number;
   updateSubFrameData: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
   triggerSubFrameFocusInRebuild: ({ sender }: BackgroundSenderParam) => void;
-  shouldRepositionSubFrameInlineMenuOnScroll: ({ sender }: BackgroundSenderParam) => void;
   destroyAutofillInlineMenuListeners: ({
     message,
     sender,

diff --git a/apps/browser/src/autofill/background/overlay.background.spec.ts b/apps/browser/src/autofill/background/overlay.background.spec.ts
@@ -629,9 +629,7 @@ describe("OverlayBackground", () => {
 
           it("skips updating the inline menu list if the user has the inline menu set to open on button click", async () => {
             inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnButtonClick);
-            jest
-              .spyOn(overlayBackground as any, "checkIsInlineMenuListVisible")
-              .mockReturnValue(false);
+            overlayBackground["inlineMenuListPort"] = null;
             tabsSendMessageSpy.mockImplementation((_tab, message, _options) => {
               if (message.command === "checkFocusedFieldHasValue") {
                 return Promise.resolve(true);
@@ -2267,7 +2265,7 @@ describe("OverlayBackground", () => {
         });
 
         it("closes the list if the user has the inline menu set to show on button click and the list is open", async () => {
-          overlayBackground["isInlineMenuListVisible"] = true;
+          overlayBackground["inlineMenuListPort"] = listPortSpy;
           inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnButtonClick);
 
           sendMockExtensionMessage({ command: "openAutofillInlineMenu" }, sender);

diff --git a/apps/browser/src/autofill/background/overlay.background.ts b/apps/browser/src/autofill/background/overlay.background.ts
@@ -168,8 +168,6 @@ export class OverlayBackground implements OverlayBackgroundInterface {
     getCurrentTabFrameId: ({ sender }) => this.getSenderFrameId(sender),
     updateSubFrameData: ({ message, sender }) => this.updateSubFrameData(message, sender),
     triggerSubFrameFocusInRebuild: ({ sender }) => this.triggerSubFrameFocusInRebuild(sender),
-    shouldRepositionSubFrameInlineMenuOnScroll: ({ sender }) =>
-      this.shouldRepositionSubFrameInlineMenuOnScroll(sender),
     destroyAutofillInlineMenuListeners: ({ message, sender }) =>
       this.triggerDestroyInlineMenuListeners(sender.tab, message.subFrameData.frameId),
     collectPageDetailsResponse: ({ message, sender }) => this.storePageDetails(message, sender),
@@ -1010,7 +1008,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
     );
 
     if (
-      !this.checkIsInlineMenuListVisible() &&
+      !this.inlineMenuListPort &&
       (await this.getInlineMenuVisibility()) === AutofillOverlayVisibility.OnButtonClick
     ) {
       return;
@@ -1819,7 +1817,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
       return;
     }
 
-    if (this.isInlineMenuListVisible) {
+    if (this.inlineMenuListPort) {
       this.closeInlineMenu(sender, {
         forceCloseInlineMenu: true,
         overlayElement: AutofillOverlayElement.List,
@@ -2600,20 +2598,6 @@ export class OverlayBackground implements OverlayBackgroundInterface {
     this.repositionInlineMenu$.next(sender);
   }
 
-  /**
-   * Triggers on scroll of a frame within the tab. Will reposition the inline menu
-   * if the focused field is within a sub-frame and the inline menu is visible.
-   *
-   * @param sender - The sender of the message
-   */
-  private shouldRepositionSubFrameInlineMenuOnScroll(sender: chrome.runtime.MessageSender) {
-    if (!this.isInlineMenuButtonVisible || sender.tab.id !== this.focusedFieldData?.tabId) {
-      return false;
-    }
-
-    return this.focusedFieldData.frameId > 0;
-  }
-
   /**
    * Handles determining if the inline menu should be repositioned or closed, and initiates
    * the process of calculating the new position of the inline menu.

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
@@ -1703,6 +1703,10 @@ describe("AutofillOverlayContentService", () => {
     const repositionEvents = [EVENTS.SCROLL, EVENTS.RESIZE];
     repositionEvents.forEach((repositionEvent) => {
       it(`sends a message trigger overlay reposition message to the background when a ${repositionEvent} event occurs`, async () => {
+        Object.defineProperty(globalThis, "scrollY", {
+          value: 10,
+          writable: true,
+        });
         sendExtensionMessageSpy.mockResolvedValueOnce(true);
         globalThis.dispatchEvent(new Event(repositionEvent));
         await flushPromises();

diff --git a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts
@@ -1568,41 +1568,46 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
    * the overlay elements on scroll or resize.
    */
   private setOverlayRepositionEventListeners() {
+    let currentScrollY = globalThis.scrollY;
+    let currentScrollX = globalThis.scrollX;
+    let mostRecentTargetScrollY = 0;
     const repositionHandler = this.useEventHandlersMemo(
       throttle(this.handleOverlayRepositionEvent, 250),
       AUTOFILL_OVERLAY_HANDLE_REPOSITION,
     );
 
-    const eventTargetContainsFocusedField = (eventTarget: Element | Document) => {
-      if (!eventTarget || !this.mostRecentlyFocusedField) {
+    const eventTargetContainsFocusedField = (eventTarget: Element) => {
+      if (typeof eventTarget.contains !== "function") {
         return false;
       }
 
-      const activeElement = (eventTarget as Document).activeElement;
-      if (activeElement) {
-        return (
-          activeElement === this.mostRecentlyFocusedField ||
-          activeElement.contains(this.mostRecentlyFocusedField) ||
-          this.inlineMenuContentService?.isElementInlineMenu(activeElement as HTMLElement)
-        );
-      }
-
-      if (typeof eventTarget.contains !== "function") {
+      const targetScrollY = eventTarget.scrollTop;
+      if (targetScrollY === mostRecentTargetScrollY) {
         return false;
       }
-      return (
+
+      if (
         eventTarget === this.mostRecentlyFocusedField ||
         eventTarget.contains(this.mostRecentlyFocusedField)
-      );
+      ) {
+        mostRecentTargetScrollY = targetScrollY;
+        return true;
+      }
+
+      return false;
     };
     const scrollHandler = this.useEventHandlersMemo(
       throttle(async (event) => {
         if (
-          eventTargetContainsFocusedField(event.target) ||
-          (await this.shouldRepositionSubFrameInlineMenuOnScroll())
+          currentScrollY !== globalThis.scrollY ||
+          currentScrollX !== globalThis.scrollX ||
+          eventTargetContainsFocusedField(event.target)
         ) {
           repositionHandler(event);
         }
+
+        currentScrollY = globalThis.scrollY;
+        currentScrollX = globalThis.scrollX;
       }, 50),
       AUTOFILL_OVERLAY_HANDLE_SCROLL,
     );