From a4c6731021a0587a1e04341d1fad5f478d706ff1 Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Fri, 1 Nov 2024 11:17:54 -0500 Subject: [PATCH] [PM-14054] Fixing scroll-based repositioning of the inline menu on ShadowDOM 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 --- .../abstractions/overlay.background.ts | 1 - .../background/overlay.background.spec.ts | 6 +-- .../autofill/background/overlay.background.ts | 20 +-------- .../autofill-overlay-content.service.spec.ts | 4 ++ .../autofill-overlay-content.service.ts | 41 +++++++++++-------- 5 files changed, 31 insertions(+), 41 deletions(-) diff --git a/apps/browser/src/autofill/background/abstractions/overlay.background.ts b/apps/browser/src/autofill/background/abstractions/overlay.background.ts index db50b784453..68d3f32b80f 100644 --- 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 index d59ed447dde..29ae35d5cef 100644 --- 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 index 41791b3b75f..a2b3e33d74f 100644 --- 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 index 49a0b3ca844..8a77534d0d4 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 @@ -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 index ea3c5784949..511e5dd594b 100644 --- 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) { - return false; - } - - const activeElement = (eventTarget as Document).activeElement; - if (activeElement) { - return ( - activeElement === this.mostRecentlyFocusedField || - activeElement.contains(this.mostRecentlyFocusedField) || - this.inlineMenuContentService?.isElementInlineMenu(activeElement as HTMLElement) - ); - } - + const eventTargetContainsFocusedField = (eventTarget: Element) => { if (typeof eventTarget.contains !== "function") { return false; } - return ( + + const targetScrollY = eventTarget.scrollTop; + if (targetScrollY === mostRecentTargetScrollY) { + return false; + } + + 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, );