From 5f0755d74d90b773fc6e70d6ead5139c3691471f Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Fri, 25 Oct 2024 16:17:36 -0500 Subject: [PATCH] [PM-14045] Scrolling content outside of iframe bounds breaks inline menu position (#11716) * [PM-14045] Scrolling content outside of iframe bounds breaks inline menu position * [PM-14045] Scrolling content outside of iframe bounds breaks inline menu position * [PM-14045] Fixing jest test * [PM-14045] Adjusting how we determine if the inline menu should reposition on scroll --- .../abstractions/overlay.background.ts | 1 + .../autofill/background/overlay.background.ts | 16 ++++++++ .../autofill-overlay-content.service.spec.ts | 1 + .../autofill-overlay-content.service.ts | 37 ++++++++++++++++--- 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/apps/browser/src/autofill/background/abstractions/overlay.background.ts b/apps/browser/src/autofill/background/abstractions/overlay.background.ts index 68d3f32b80..db50b78445 100644 --- a/apps/browser/src/autofill/background/abstractions/overlay.background.ts +++ b/apps/browser/src/autofill/background/abstractions/overlay.background.ts @@ -216,6 +216,7 @@ 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.ts b/apps/browser/src/autofill/background/overlay.background.ts index 2b8f2c273c..6fb4589baa 100644 --- a/apps/browser/src/autofill/background/overlay.background.ts +++ b/apps/browser/src/autofill/background/overlay.background.ts @@ -168,6 +168,8 @@ 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), @@ -2594,6 +2596,20 @@ 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 index c74fa21937..91ad63955c 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 @@ -1699,6 +1699,7 @@ 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 () => { + 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 index 760a585bd6..645795d9f2 100644 --- a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts +++ b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts @@ -1571,14 +1571,35 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ AUTOFILL_OVERLAY_HANDLE_REPOSITION, ); - const eventTargetDoesNotContainFocusedField = (element: Element) => - typeof element?.contains === "function" && !element.contains(this.mostRecentlyFocusedField); + const eventTargetContainsFocusedField = (eventTarget: Element | Document) => { + if (!eventTarget || !this.mostRecentlyFocusedField) { + return false; + } + + const activeElement = (eventTarget as Document).activeElement; + if (activeElement) { + return ( + activeElement === this.mostRecentlyFocusedField || + activeElement.contains(this.mostRecentlyFocusedField) + ); + } + + if (typeof eventTarget.contains !== "function") { + return false; + } + return ( + eventTarget === this.mostRecentlyFocusedField || + eventTarget.contains(this.mostRecentlyFocusedField) + ); + }; const scrollHandler = this.useEventHandlersMemo( - throttle((event) => { - if (eventTargetDoesNotContainFocusedField(event.target as Element)) { - return; + throttle(async (event) => { + if ( + eventTargetContainsFocusedField(event.target) || + (await this.shouldRepositionSubFrameInlineMenuOnScroll()) + ) { + repositionHandler(event); } - repositionHandler(event); }, 50), AUTOFILL_OVERLAY_HANDLE_SCROLL, ); @@ -1590,6 +1611,10 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ globalThis.addEventListener(EVENTS.RESIZE, repositionHandler); } + private shouldRepositionSubFrameInlineMenuOnScroll = async () => { + return await this.sendExtensionMessage("shouldRepositionSubFrameInlineMenuOnScroll"); + }; + /** * Removes the listeners that facilitate repositioning * the overlay elements on scroll or resize.