From fdcf1c7ea25ea2e6562fb25b8ba8b4f77c2d5c97 Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Mon, 5 Aug 2024 11:32:29 -0500 Subject: [PATCH] [PM-10554] Inline menu glitches on single form fields that are wrapped within an iframe (#10384) * [PM-10554] Inline menu glitches on single form fields that are wrapped within inline menu * [PM-10554] Adding jest tests to validate expected behavior * [PM-10554] Adding jest tests to validate expected behavior --- .../abstractions/overlay.background.ts | 2 +- .../background/overlay.background.spec.ts | 30 ++++++++++++++++--- .../autofill/background/overlay.background.ts | 27 ++++++++++++++--- .../autofill-overlay-content.service.spec.ts | 13 ++++++++ .../autofill-overlay-content.service.ts | 11 +++++-- 5 files changed, 71 insertions(+), 12 deletions(-) diff --git a/apps/browser/src/autofill/background/abstractions/overlay.background.ts b/apps/browser/src/autofill/background/abstractions/overlay.background.ts index 401196b256..57eb391e4d 100644 --- a/apps/browser/src/autofill/background/abstractions/overlay.background.ts +++ b/apps/browser/src/autofill/background/abstractions/overlay.background.ts @@ -161,7 +161,7 @@ export type OverlayBackgroundExtensionMessageHandlers = { triggerAutofillOverlayReposition: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; checkIsInlineMenuCiphersPopulated: ({ sender }: BackgroundSenderParam) => void; updateFocusedFieldData: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; - updateIsFieldCurrentlyFocused: ({ message }: BackgroundMessageParam) => void; + updateIsFieldCurrentlyFocused: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; checkIsFieldCurrentlyFocused: () => boolean; updateIsFieldCurrentlyFilling: ({ message }: BackgroundMessageParam) => void; checkIsFieldCurrentlyFilling: () => boolean; diff --git a/apps/browser/src/autofill/background/overlay.background.spec.ts b/apps/browser/src/autofill/background/overlay.background.spec.ts index 783010ee04..725c91510d 100644 --- a/apps/browser/src/autofill/background/overlay.background.spec.ts +++ b/apps/browser/src/autofill/background/overlay.background.spec.ts @@ -526,10 +526,13 @@ describe("OverlayBackground", () => { }); it("skips updating the position of either inline menu element if a field is not currently focused", async () => { - sendMockExtensionMessage({ - command: "updateIsFieldCurrentlyFocused", - isFieldCurrentlyFocused: false, - }); + sendMockExtensionMessage( + { + command: "updateIsFieldCurrentlyFocused", + isFieldCurrentlyFocused: false, + }, + mock({ frameId: 20 }), + ); sendMockExtensionMessage({ command: "triggerAutofillOverlayReposition" }, sender); await flushUpdateInlineMenuPromises(); @@ -1362,6 +1365,25 @@ describe("OverlayBackground", () => { }); }); + describe("updateIsFieldCurrentlyFocused message handler", () => { + it("skips updating the isFiledCurrentlyFocused value when the focused field data is populated and the sender frame id does not equal the focused field's frame id", async () => { + const focusedFieldData = createFocusedFieldDataMock(); + sendMockExtensionMessage( + { command: "updateFocusedFieldData", focusedFieldData }, + mock({ tab: { id: 1 }, frameId: 10 }), + ); + overlayBackground["isFieldCurrentlyFocused"] = true; + + sendMockExtensionMessage( + { command: "updateIsFieldCurrentlyFocused", isFieldCurrentlyFocused: false }, + mock({ tab: { id: 1 }, frameId: 20 }), + ); + await flushPromises(); + + expect(overlayBackground["isFieldCurrentlyFocused"]).toBe(true); + }); + }); + describe("checkIsFieldCurrentlyFocused message handler", () => { it("returns true when a form field is currently focused", async () => { sendMockExtensionMessage({ diff --git a/apps/browser/src/autofill/background/overlay.background.ts b/apps/browser/src/autofill/background/overlay.background.ts index 029d2d69ac..ed70a14c4a 100644 --- a/apps/browser/src/autofill/background/overlay.background.ts +++ b/apps/browser/src/autofill/background/overlay.background.ts @@ -97,7 +97,8 @@ export class OverlayBackground implements OverlayBackgroundInterface { checkIsInlineMenuCiphersPopulated: ({ sender }) => this.checkIsInlineMenuCiphersPopulated(sender), updateFocusedFieldData: ({ message, sender }) => this.setFocusedFieldData(message, sender), - updateIsFieldCurrentlyFocused: ({ message }) => this.updateIsFieldCurrentlyFocused(message), + updateIsFieldCurrentlyFocused: ({ message, sender }) => + this.updateIsFieldCurrentlyFocused(message, sender), checkIsFieldCurrentlyFocused: () => this.checkIsFieldCurrentlyFocused(), updateIsFieldCurrentlyFilling: ({ message }) => this.updateIsFieldCurrentlyFilling(message), checkIsFieldCurrentlyFilling: () => this.checkIsFieldCurrentlyFilling(), @@ -1090,7 +1091,7 @@ export class OverlayBackground implements OverlayBackgroundInterface { { focusedFieldData }: OverlayBackgroundExtensionMessage, sender: chrome.runtime.MessageSender, ) { - if (this.focusedFieldData?.frameId && this.focusedFieldData.frameId !== sender.frameId) { + if (this.focusedFieldData && !this.senderFrameHasFocusedField(sender)) { void BrowserApi.tabSendMessage( sender.tab, { command: "unsetMostRecentlyFocusedField" }, @@ -1100,6 +1101,7 @@ export class OverlayBackground implements OverlayBackgroundInterface { const previousFocusedFieldData = this.focusedFieldData; this.focusedFieldData = { ...focusedFieldData, tabId: sender.tab.id, frameId: sender.frameId }; + this.isFieldCurrentlyFocused = true; const accountCreationFieldBlurred = previousFocusedFieldData?.showInlineMenuAccountCreation && @@ -1558,8 +1560,16 @@ export class OverlayBackground implements OverlayBackgroundInterface { * Updates the property that identifies if a form field set up for the inline menu is currently focused. * * @param message - The message received from the web page + * @param sender - The sender of the port message */ - private updateIsFieldCurrentlyFocused(message: OverlayBackgroundExtensionMessage) { + private updateIsFieldCurrentlyFocused( + message: OverlayBackgroundExtensionMessage, + sender: chrome.runtime.MessageSender, + ) { + if (this.focusedFieldData && !this.senderFrameHasFocusedField(sender)) { + return; + } + this.isFieldCurrentlyFocused = message.isFieldCurrentlyFocused; } @@ -1651,7 +1661,7 @@ export class OverlayBackground implements OverlayBackgroundInterface { return false; } - if (this.focusedFieldData?.frameId === sender.frameId) { + if (this.senderFrameHasFocusedField(sender)) { return true; } @@ -1676,6 +1686,15 @@ export class OverlayBackground implements OverlayBackgroundInterface { return sender.tab.id === this.focusedFieldData?.tabId; } + /** + * Identifies if the sender frame is the same as the focused field's frame. + * + * @param sender - The sender of the message + */ + private senderFrameHasFocusedField(sender: chrome.runtime.MessageSender) { + return sender.frameId === this.focusedFieldData?.frameId; + } + /** * Triggers when a scroll or resize event occurs within a tab. Will reposition the inline menu * if the focused field is within the viewport. 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 6145dbfdc0..15eeff75cd 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 @@ -1986,6 +1986,19 @@ describe("AutofillOverlayContentService", () => { expect(autofillFieldFocusSpy).not.toHaveBeenCalled(); expect(nextFocusableElement.focus).toHaveBeenCalled(); }); + + it("focuses the most recently focused input field if no other tabbable elements are found", async () => { + autofillOverlayContentService["focusableElements"] = []; + findTabsSpy.mockReturnValue([]); + + sendMockExtensionMessage({ + command: "redirectAutofillInlineMenuFocusOut", + data: { direction: RedirectFocusDirection.Next }, + }); + await flushPromises(); + + expect(autofillFieldFocusSpy).toHaveBeenCalled(); + }); }); describe("updateAutofillInlineMenuVisibility message handler", () => { 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 0881ecba1e..9a45403edc 100644 --- a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts +++ b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts @@ -338,7 +338,12 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ const indexOffset = direction === RedirectFocusDirection.Previous ? -1 : 1; const redirectFocusElement = this.focusableElements[focusedElementIndex + indexOffset]; - redirectFocusElement?.focus(); + if (redirectFocusElement) { + redirectFocusElement.focus(); + return; + } + + this.focusMostRecentlyFocusedField(); } /** @@ -1418,8 +1423,8 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ focusedFieldRectsTop + this.focusedFieldData?.focusedFieldRects?.height; const viewportHeight = globalThis.innerHeight + globalThis.scrollY; return ( - focusedFieldRectsTop && - focusedFieldRectsTop > 0 && + !globalThis.isNaN(focusedFieldRectsTop) && + focusedFieldRectsTop >= 0 && focusedFieldRectsTop < viewportHeight && focusedFieldRectsBottom < viewportHeight );