[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
This commit is contained in:
Cesar Gonzalez 2024-08-05 11:32:29 -05:00 committed by GitHub
parent 7e3358d4ee
commit fdcf1c7ea2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 71 additions and 12 deletions

View File

@ -161,7 +161,7 @@ export type OverlayBackgroundExtensionMessageHandlers = {
triggerAutofillOverlayReposition: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; triggerAutofillOverlayReposition: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
checkIsInlineMenuCiphersPopulated: ({ sender }: BackgroundSenderParam) => void; checkIsInlineMenuCiphersPopulated: ({ sender }: BackgroundSenderParam) => void;
updateFocusedFieldData: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; updateFocusedFieldData: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
updateIsFieldCurrentlyFocused: ({ message }: BackgroundMessageParam) => void; updateIsFieldCurrentlyFocused: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
checkIsFieldCurrentlyFocused: () => boolean; checkIsFieldCurrentlyFocused: () => boolean;
updateIsFieldCurrentlyFilling: ({ message }: BackgroundMessageParam) => void; updateIsFieldCurrentlyFilling: ({ message }: BackgroundMessageParam) => void;
checkIsFieldCurrentlyFilling: () => boolean; checkIsFieldCurrentlyFilling: () => boolean;

View File

@ -526,10 +526,13 @@ describe("OverlayBackground", () => {
}); });
it("skips updating the position of either inline menu element if a field is not currently focused", async () => { it("skips updating the position of either inline menu element if a field is not currently focused", async () => {
sendMockExtensionMessage({ sendMockExtensionMessage(
{
command: "updateIsFieldCurrentlyFocused", command: "updateIsFieldCurrentlyFocused",
isFieldCurrentlyFocused: false, isFieldCurrentlyFocused: false,
}); },
mock<chrome.runtime.MessageSender>({ frameId: 20 }),
);
sendMockExtensionMessage({ command: "triggerAutofillOverlayReposition" }, sender); sendMockExtensionMessage({ command: "triggerAutofillOverlayReposition" }, sender);
await flushUpdateInlineMenuPromises(); 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<chrome.runtime.MessageSender>({ tab: { id: 1 }, frameId: 10 }),
);
overlayBackground["isFieldCurrentlyFocused"] = true;
sendMockExtensionMessage(
{ command: "updateIsFieldCurrentlyFocused", isFieldCurrentlyFocused: false },
mock<chrome.runtime.MessageSender>({ tab: { id: 1 }, frameId: 20 }),
);
await flushPromises();
expect(overlayBackground["isFieldCurrentlyFocused"]).toBe(true);
});
});
describe("checkIsFieldCurrentlyFocused message handler", () => { describe("checkIsFieldCurrentlyFocused message handler", () => {
it("returns true when a form field is currently focused", async () => { it("returns true when a form field is currently focused", async () => {
sendMockExtensionMessage({ sendMockExtensionMessage({

View File

@ -97,7 +97,8 @@ export class OverlayBackground implements OverlayBackgroundInterface {
checkIsInlineMenuCiphersPopulated: ({ sender }) => checkIsInlineMenuCiphersPopulated: ({ sender }) =>
this.checkIsInlineMenuCiphersPopulated(sender), this.checkIsInlineMenuCiphersPopulated(sender),
updateFocusedFieldData: ({ message, sender }) => this.setFocusedFieldData(message, sender), updateFocusedFieldData: ({ message, sender }) => this.setFocusedFieldData(message, sender),
updateIsFieldCurrentlyFocused: ({ message }) => this.updateIsFieldCurrentlyFocused(message), updateIsFieldCurrentlyFocused: ({ message, sender }) =>
this.updateIsFieldCurrentlyFocused(message, sender),
checkIsFieldCurrentlyFocused: () => this.checkIsFieldCurrentlyFocused(), checkIsFieldCurrentlyFocused: () => this.checkIsFieldCurrentlyFocused(),
updateIsFieldCurrentlyFilling: ({ message }) => this.updateIsFieldCurrentlyFilling(message), updateIsFieldCurrentlyFilling: ({ message }) => this.updateIsFieldCurrentlyFilling(message),
checkIsFieldCurrentlyFilling: () => this.checkIsFieldCurrentlyFilling(), checkIsFieldCurrentlyFilling: () => this.checkIsFieldCurrentlyFilling(),
@ -1090,7 +1091,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
{ focusedFieldData }: OverlayBackgroundExtensionMessage, { focusedFieldData }: OverlayBackgroundExtensionMessage,
sender: chrome.runtime.MessageSender, sender: chrome.runtime.MessageSender,
) { ) {
if (this.focusedFieldData?.frameId && this.focusedFieldData.frameId !== sender.frameId) { if (this.focusedFieldData && !this.senderFrameHasFocusedField(sender)) {
void BrowserApi.tabSendMessage( void BrowserApi.tabSendMessage(
sender.tab, sender.tab,
{ command: "unsetMostRecentlyFocusedField" }, { command: "unsetMostRecentlyFocusedField" },
@ -1100,6 +1101,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
const previousFocusedFieldData = this.focusedFieldData; const previousFocusedFieldData = this.focusedFieldData;
this.focusedFieldData = { ...focusedFieldData, tabId: sender.tab.id, frameId: sender.frameId }; this.focusedFieldData = { ...focusedFieldData, tabId: sender.tab.id, frameId: sender.frameId };
this.isFieldCurrentlyFocused = true;
const accountCreationFieldBlurred = const accountCreationFieldBlurred =
previousFocusedFieldData?.showInlineMenuAccountCreation && 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. * 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 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; this.isFieldCurrentlyFocused = message.isFieldCurrentlyFocused;
} }
@ -1651,7 +1661,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
return false; return false;
} }
if (this.focusedFieldData?.frameId === sender.frameId) { if (this.senderFrameHasFocusedField(sender)) {
return true; return true;
} }
@ -1676,6 +1686,15 @@ export class OverlayBackground implements OverlayBackgroundInterface {
return sender.tab.id === this.focusedFieldData?.tabId; 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 * Triggers when a scroll or resize event occurs within a tab. Will reposition the inline menu
* if the focused field is within the viewport. * if the focused field is within the viewport.

View File

@ -1986,6 +1986,19 @@ describe("AutofillOverlayContentService", () => {
expect(autofillFieldFocusSpy).not.toHaveBeenCalled(); expect(autofillFieldFocusSpy).not.toHaveBeenCalled();
expect(nextFocusableElement.focus).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", () => { describe("updateAutofillInlineMenuVisibility message handler", () => {

View File

@ -338,7 +338,12 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
const indexOffset = direction === RedirectFocusDirection.Previous ? -1 : 1; const indexOffset = direction === RedirectFocusDirection.Previous ? -1 : 1;
const redirectFocusElement = this.focusableElements[focusedElementIndex + indexOffset]; 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; focusedFieldRectsTop + this.focusedFieldData?.focusedFieldRects?.height;
const viewportHeight = globalThis.innerHeight + globalThis.scrollY; const viewportHeight = globalThis.innerHeight + globalThis.scrollY;
return ( return (
focusedFieldRectsTop && !globalThis.isNaN(focusedFieldRectsTop) &&
focusedFieldRectsTop > 0 && focusedFieldRectsTop >= 0 &&
focusedFieldRectsTop < viewportHeight && focusedFieldRectsTop < viewportHeight &&
focusedFieldRectsBottom < viewportHeight focusedFieldRectsBottom < viewportHeight
); );