diff --git a/apps/browser/src/autofill/popup/settings/autofill.component.ts b/apps/browser/src/autofill/popup/settings/autofill.component.ts index 67cc25f227..1c6583331f 100644 --- a/apps/browser/src/autofill/popup/settings/autofill.component.ts +++ b/apps/browser/src/autofill/popup/settings/autofill.component.ts @@ -105,11 +105,7 @@ export class AutofillComponent implements OnInit { } async updateAutoFillOverlayVisibility() { - const previousAutoFillOverlayVisibility = await firstValueFrom( - this.autofillSettingsService.inlineMenuVisibility$, - ); await this.autofillSettingsService.setInlineMenuVisibility(this.autoFillOverlayVisibility); - await this.handleUpdatingAutofillOverlayContentScripts(previousAutoFillOverlayVisibility); await this.requestPrivacyPermission(); } @@ -181,27 +177,6 @@ export class AutofillComponent implements OnInit { BrowserApi.createNewTab(this.disablePasswordManagerLink); } - private async handleUpdatingAutofillOverlayContentScripts( - previousAutoFillOverlayVisibility: number, - ) { - const autofillOverlayPreviouslyDisabled = - previousAutoFillOverlayVisibility === AutofillOverlayVisibility.Off; - const autofillOverlayCurrentlyDisabled = - this.autoFillOverlayVisibility === AutofillOverlayVisibility.Off; - - if (!autofillOverlayPreviouslyDisabled && !autofillOverlayCurrentlyDisabled) { - const tabs = await BrowserApi.tabsQuery({}); - tabs.forEach((tab) => - BrowserApi.tabSendMessageData(tab, "updateAutofillOverlayVisibility", { - autofillOverlayVisibility: this.autoFillOverlayVisibility, - }), - ); - return; - } - - await this.autofillService.reloadAutofillScripts(); - } - async requestPrivacyPermission() { if ( this.autoFillOverlayVisibility === AutofillOverlayVisibility.Off || diff --git a/apps/browser/src/autofill/services/autofill.service.spec.ts b/apps/browser/src/autofill/services/autofill.service.spec.ts index 158fde6a56..24b1c90b3f 100644 --- a/apps/browser/src/autofill/services/autofill.service.spec.ts +++ b/apps/browser/src/autofill/services/autofill.service.spec.ts @@ -1,5 +1,5 @@ -import { mock, mockReset } from "jest-mock-extended"; -import { of } from "rxjs"; +import { mock, mockReset, MockProxy } from "jest-mock-extended"; +import { BehaviorSubject, of } from "rxjs"; import { UserVerificationService } from "@bitwarden/common/auth/services/user-verification/user-verification.service"; import { AutofillOverlayVisibility } from "@bitwarden/common/autofill/constants"; @@ -8,6 +8,7 @@ import { DefaultDomainSettingsService, DomainSettingsService, } from "@bitwarden/common/autofill/services/domain-settings.service"; +import { InlineMenuVisibilitySetting } from "@bitwarden/common/autofill/types"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EventType } from "@bitwarden/common/enums"; import { UriMatchStrategy } from "@bitwarden/common/models/domain/domain-service"; @@ -45,7 +46,7 @@ import { createChromeTabMock, createGenerateFillScriptOptionsMock, } from "../spec/autofill-mocks"; -import { triggerTestFailure } from "../spec/testing-utils"; +import { flushPromises, triggerTestFailure } from "../spec/testing-utils"; import { AutoFillOptions, @@ -64,7 +65,8 @@ const mockEquivalentDomains = [ describe("AutofillService", () => { let autofillService: AutofillService; const cipherService = mock(); - const autofillSettingsService = mock(); + let inlineMenuVisibilityMock$!: BehaviorSubject; + let autofillSettingsService: MockProxy; const mockUserId = Utils.newGuid() as UserId; const accountService: FakeAccountService = mockAccountServiceWith(mockUserId); const fakeStateProvider: FakeStateProvider = new FakeStateProvider(accountService); @@ -79,6 +81,9 @@ describe("AutofillService", () => { beforeEach(() => { scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService); + inlineMenuVisibilityMock$ = new BehaviorSubject(AutofillOverlayVisibility.OnFieldFocus); + autofillSettingsService = mock(); + (autofillSettingsService as any).inlineMenuVisibility$ = inlineMenuVisibilityMock$; autofillService = new AutofillService( cipherService, autofillSettingsService, @@ -142,17 +147,92 @@ describe("AutofillService", () => { // eslint-disable-next-line no-restricted-syntax expect(chrome.runtime.onConnect.addListener).toHaveBeenCalledWith(expect.any(Function)); }); + + describe("handle inline menu visibility change", () => { + beforeEach(async () => { + await autofillService.loadAutofillScriptsOnInstall(); + jest.spyOn(BrowserApi, "tabsQuery").mockResolvedValue([tab1, tab2]); + jest.spyOn(BrowserApi, "tabSendMessageData").mockImplementation(); + jest.spyOn(autofillService, "reloadAutofillScripts").mockImplementation(); + }); + + it("returns early if the setting is being initialized", async () => { + await flushPromises(); + + expect(BrowserApi.tabsQuery).toHaveBeenCalledTimes(1); + expect(BrowserApi.tabSendMessageData).not.toHaveBeenCalled(); + }); + + it("returns early if the previous setting is equivalent to the new setting", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus); + await flushPromises(); + + expect(BrowserApi.tabsQuery).toHaveBeenCalledTimes(1); + expect(BrowserApi.tabSendMessageData).not.toHaveBeenCalled(); + }); + + describe("updates the inline menu visibility setting", () => { + it("when changing the inline menu from on focus of field to on button click", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnButtonClick); + await flushPromises(); + + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + tab1, + "updateAutofillOverlayVisibility", + { autofillOverlayVisibility: AutofillOverlayVisibility.OnButtonClick }, + ); + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + tab2, + "updateAutofillOverlayVisibility", + { autofillOverlayVisibility: AutofillOverlayVisibility.OnButtonClick }, + ); + }); + + it("when changing the inline menu from button click to field focus", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnButtonClick); + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus); + await flushPromises(); + + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + tab1, + "updateAutofillOverlayVisibility", + { autofillOverlayVisibility: AutofillOverlayVisibility.OnFieldFocus }, + ); + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + tab2, + "updateAutofillOverlayVisibility", + { autofillOverlayVisibility: AutofillOverlayVisibility.OnFieldFocus }, + ); + }); + }); + + describe("reloads the autofill scripts", () => { + it("when changing the inline menu from a disabled setting to an enabled setting", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.Off); + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus); + await flushPromises(); + + expect(autofillService.reloadAutofillScripts).toHaveBeenCalled(); + }); + + it("when changing the inline menu from a enabled setting to a disabled setting", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus); + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.Off); + await flushPromises(); + + expect(autofillService.reloadAutofillScripts).toHaveBeenCalled(); + }); + }); + }); }); describe("reloadAutofillScripts", () => { - it("disconnects and removes all autofill script ports", () => { - const port1 = mock({ - disconnect: jest.fn(), - }); - const port2 = mock({ - disconnect: jest.fn(), - }); + it("re-injects the autofill scripts in all tabs and disconnects all connected ports", () => { + const port1 = mock(); + const port2 = mock(); autofillService["autofillScriptPortsSet"] = new Set([port1, port2]); + jest.spyOn(autofillService as any, "injectAutofillScriptsInAllTabs"); + jest.spyOn(autofillService, "getAutofillOnPageLoad").mockResolvedValue(true); // 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 @@ -161,17 +241,6 @@ describe("AutofillService", () => { expect(port1.disconnect).toHaveBeenCalled(); expect(port2.disconnect).toHaveBeenCalled(); expect(autofillService["autofillScriptPortsSet"].size).toBe(0); - }); - - it("re-injects the autofill scripts in all tabs", () => { - autofillService["autofillScriptPortsSet"] = new Set([mock()]); - jest.spyOn(autofillService as any, "injectAutofillScriptsInAllTabs"); - jest.spyOn(autofillService, "getAutofillOnPageLoad").mockResolvedValue(true); - - // 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 - autofillService.reloadAutofillScripts(); - expect(autofillService["injectAutofillScriptsInAllTabs"]).toHaveBeenCalled(); }); }); diff --git a/apps/browser/src/autofill/services/autofill.service.ts b/apps/browser/src/autofill/services/autofill.service.ts index 80829ee714..5348ca5b9a 100644 --- a/apps/browser/src/autofill/services/autofill.service.ts +++ b/apps/browser/src/autofill/services/autofill.service.ts @@ -1,4 +1,5 @@ -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, startWith } from "rxjs"; +import { pairwise } from "rxjs/operators"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; @@ -70,10 +71,12 @@ export default class AutofillService implements AutofillServiceInterface { */ async loadAutofillScriptsOnInstall() { BrowserApi.addListener(chrome.runtime.onConnect, this.handleInjectedScriptPortConnection); - - // 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.injectAutofillScriptsInAllTabs(); + void this.injectAutofillScriptsInAllTabs(); + this.autofillSettingsService.inlineMenuVisibility$ + .pipe(startWith(undefined), pairwise()) + .subscribe(([previousSetting, currentSetting]) => + this.handleInlineMenuVisibilityChange(previousSetting, currentSetting), + ); } /** @@ -2090,4 +2093,34 @@ export default class AutofillService implements AutofillServiceInterface { } } } + + /** + * Updates the autofill inline menu visibility setting in all active tabs + * when the InlineMenuVisibilitySetting observable is updated. + * + * @param previousSetting - The previous setting value + * @param currentSetting - The current setting value + */ + private async handleInlineMenuVisibilityChange( + previousSetting: InlineMenuVisibilitySetting, + currentSetting: InlineMenuVisibilitySetting, + ) { + if (previousSetting === undefined || previousSetting === currentSetting) { + return; + } + + const inlineMenuPreviouslyDisabled = previousSetting === AutofillOverlayVisibility.Off; + const inlineMenuCurrentlyDisabled = currentSetting === AutofillOverlayVisibility.Off; + if (!inlineMenuPreviouslyDisabled && !inlineMenuCurrentlyDisabled) { + const tabs = await BrowserApi.tabsQuery({}); + tabs.forEach((tab) => + BrowserApi.tabSendMessageData(tab, "updateAutofillOverlayVisibility", { + autofillOverlayVisibility: currentSetting, + }), + ); + return; + } + + await this.reloadAutofillScripts(); + } } diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.ts index 7c49a3d988..909d858430 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.ts @@ -1349,6 +1349,10 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte } const cachedAutofillFieldElement = this.autofillFieldElements.get(formFieldElement); + if (!cachedAutofillFieldElement) { + continue; + } + cachedAutofillFieldElement.viewable = true; void this.autofillOverlayContentService?.setupAutofillOverlayListenerOnField( diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index 5473664e3b..388060d88c 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -188,6 +188,7 @@ export default class RuntimeBackground { if (msg.command === "loggedIn") { await this.sendBwInstalledMessageToVault(); + await this.autofillService.reloadAutofillScripts(); } if (this.lockedVaultPendingNotifications?.length > 0) {