From 73504d9bb0303157bc34e69a8a10c605585a7ead Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Thu, 7 Mar 2024 14:37:16 -0600 Subject: [PATCH] [PM-6469] Fix inline menu UI visibility race condition bug (#8104) --- .../autofill-overlay-content.service.spec.ts | 33 +++++++++++++++++-- .../autofill-overlay-content.service.ts | 8 ++--- 2 files changed, 34 insertions(+), 7 deletions(-) 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 4b3641208b..8926f5b298 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 @@ -1214,7 +1214,10 @@ describe("AutofillOverlayContentService", () => { autofillOverlayContentService as any, "removeAutofillOverlay", ); + autofillOverlayContentService["mostRecentlyFocusedField"] = undefined; + autofillOverlayContentService["overlayButtonElement"] = document.createElement("div"); + autofillOverlayContentService["overlayListElement"] = document.createElement("div"); globalThis.dispatchEvent(new Event(EVENTS.SCROLL)); jest.advanceTimersByTime(800); @@ -1222,8 +1225,8 @@ describe("AutofillOverlayContentService", () => { expect(sendExtensionMessageSpy).toHaveBeenCalledWith("updateAutofillOverlayHidden", { display: "block", }); - expect(autofillOverlayContentService["isOverlayButtonVisible"]).toEqual(true); - expect(autofillOverlayContentService["isOverlayListVisible"]).toEqual(true); + expect(autofillOverlayContentService["isOverlayButtonVisible"]).toEqual(false); + expect(autofillOverlayContentService["isOverlayListVisible"]).toEqual(false); expect(removeAutofillOverlaySpy).toHaveBeenCalled(); }); @@ -1280,6 +1283,32 @@ describe("AutofillOverlayContentService", () => { expect(removeAutofillOverlaySpy).toHaveBeenCalled(); }); + + it("defaults overlay elements to a visibility of `false` if the element is not rendered on the page", async () => { + jest.useFakeTimers(); + jest + .spyOn(autofillOverlayContentService as any, "updateMostRecentlyFocusedField") + .mockImplementation(() => { + autofillOverlayContentService["focusedFieldData"] = { + focusedFieldRects: { + top: 100, + }, + focusedFieldStyles: {}, + }; + }); + jest + .spyOn(autofillOverlayContentService as any, "updateOverlayElementsPosition") + .mockImplementation(); + autofillOverlayContentService["overlayButtonElement"] = document.createElement("div"); + autofillOverlayContentService["overlayListElement"] = undefined; + + globalThis.dispatchEvent(new Event(EVENTS.SCROLL)); + jest.advanceTimersByTime(800); + await flushPromises(); + + expect(autofillOverlayContentService["isOverlayButtonVisible"]).toEqual(true); + expect(autofillOverlayContentService["isOverlayListVisible"]).toEqual(false); + }); }); describe("handleOverlayElementMutationObserverUpdate", () => { 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 efbc9732b6..2cf063a5ba 100644 --- a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts +++ b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts @@ -646,12 +646,10 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte */ private toggleOverlayHidden(isHidden: boolean) { const displayValue = isHidden ? "none" : "block"; - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.sendExtensionMessage("updateAutofillOverlayHidden", { display: displayValue }); + void this.sendExtensionMessage("updateAutofillOverlayHidden", { display: displayValue }); - this.isOverlayButtonVisible = !isHidden; - this.isOverlayListVisible = !isHidden; + this.isOverlayButtonVisible = !!this.overlayButtonElement && !isHidden; + this.isOverlayListVisible = !!this.overlayListElement && !isHidden; } /**