[PM-29209] Fix persistent browser settings berry (#18113)

* [PM-29209] Introduce new autofill nudge service specific to the Browser client

* [PM-29209] Cleanup redundant browser setting checks

* [PM-29209] Ensure nudge is dismissed on nudge button click

* [PM-29209] Add spec file for browser autofill nudge service

* [PM-29209] Cleanup settings-v2 spec file

apps/browser/src/autofill/popup/settings/autofill.component.html

@@ -6,16 +6,18 @@
   </popup-header>
 
   <div class="tw-bg-background-alt">
-    <div *ngIf="!defaultBrowserAutofillDisabled && (showSpotlightNudge$ | async)" class="tw-mb-6">
-      <bit-spotlight
-        [title]="'autofillSpotlightTitle' | i18n"
-        [subtitle]="'autofillSpotlightDesc' | i18n"
-        [buttonText]="spotlightButtonText"
-        (onDismiss)="dismissSpotlight()"
-        (onButtonClick)="disableBrowserAutofillSettingsFromNudge($event)"
-        [buttonIcon]="spotlightButtonIcon"
-      ></bit-spotlight>
-    </div>
+    @if (showSpotlightNudge$ | async) {
+      <div class="tw-mb-6">
+        <bit-spotlight
+          [title]="'autofillSpotlightTitle' | i18n"
+          [subtitle]="'autofillSpotlightDesc' | i18n"
+          [buttonText]="spotlightButtonText"
+          (onDismiss)="dismissSpotlight()"
+          (onButtonClick)="disableBrowserAutofillSettingsFromNudge($event)"
+          [buttonIcon]="spotlightButtonIcon"
+        ></bit-spotlight>
+      </div>
+    }
     <bit-section>
       <bit-section-header>
         <h2 bitTypography="h6">{{ "autofillSuggestionsSectionTitle" | i18n }}</h2>

apps/browser/src/autofill/popup/settings/autofill.component.ts

@@ -611,6 +611,10 @@ export class AutofillComponent implements OnInit {
     if (this.canOverrideBrowserAutofillSetting) {
       this.defaultBrowserAutofillDisabled = true;
       await this.updateDefaultBrowserAutofillDisabled();
+      await this.nudgesService.dismissNudge(
+        NudgeType.AutofillNudge,
+        await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)),
+      );
     } else {
       await this.openURI(event, this.disablePasswordManagerURI);
     }

apps/browser/src/popup/services/services.module.ts

@@ -23,6 +23,8 @@ import {
   WINDOW,
 } from "@bitwarden/angular/services/injection-tokens";
 import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module";
+import { AUTOFILL_NUDGE_SERVICE } from "@bitwarden/angular/vault";
+import { SingleNudgeService } from "@bitwarden/angular/vault/services/default-single-nudge.service";
 import {
   LoginComponentService,
   TwoFactorAuthComponentService,
@@ -208,6 +210,7 @@ import {
 } from "../../platform/system-notifications/browser-system-notification.service";
 import { fromChromeRuntimeMessaging } from "../../platform/utils/from-chrome-runtime-messaging";
 import { FilePopoutUtilsService } from "../../tools/popup/services/file-popout-utils.service";
+import { BrowserAutofillNudgeService } from "../../vault/popup/services/browser-autofill-nudge.service";
 import { Fido2UserVerificationService } from "../../vault/services/fido2-user-verification.service";
 import { ExtensionAnonLayoutWrapperDataService } from "../components/extension-anon-layout-wrapper/extension-anon-layout-wrapper-data.service";
 
@@ -756,6 +759,11 @@ const safeProviders: SafeProvider[] = [
       MessagingServiceAbstraction,
     ],
   }),
+  safeProvider({
+    provide: AUTOFILL_NUDGE_SERVICE as SafeInjectionToken<SingleNudgeService>,
+    useClass: BrowserAutofillNudgeService,
+    deps: [],
+  }),
 ];
 
 @NgModule({

apps/browser/src/tools/popup/settings/settings-v2.component.html

@@ -34,13 +34,11 @@
         <i slot="start" class="bwi bwi-check-circle" aria-hidden="true"></i>
         <div class="tw-flex tw-items-center tw-justify-center">
           <p class="tw-pr-2">{{ "autofill" | i18n }}</p>
-          <span
-            *ngIf="!(isBrowserAutofillSettingOverridden$ | async) && (showAutofillBadge$ | async)"
-            bitBadge
-            variant="notification"
-            [attr.aria-label]="'nudgeBadgeAria' | i18n"
-            >1</span
-          >
+          @if (showAutofillBadge$ | async) {
+            <span bitBadge variant="notification" [attr.aria-label]="'nudgeBadgeAria' | i18n"
+              >1</span
+            >
+          }
         </div>
         <i slot="end" class="bwi bwi-angle-right" aria-hidden="true"></i>
       </a>

apps/browser/src/tools/popup/settings/settings-v2.component.spec.ts

@@ -148,31 +148,7 @@ describe("SettingsV2Component", () => {
     expect(openSpy).toHaveBeenCalledWith(dialogService);
   });
 
-  it("isBrowserAutofillSettingOverridden$ emits the value from the AutofillBrowserSettingsService", async () => {
-    pushActiveAccount();
-
-    mockAutofillSettings.isBrowserAutofillSettingOverridden.mockResolvedValue(true);
-
-    const fixture = TestBed.createComponent(SettingsV2Component);
-    const component = fixture.componentInstance;
-    fixture.detectChanges();
-    await fixture.whenStable();
-
-    const value = await firstValueFrom(component["isBrowserAutofillSettingOverridden$"]);
-    expect(value).toBe(true);
-
-    mockAutofillSettings.isBrowserAutofillSettingOverridden.mockResolvedValue(false);
-
-    const fixture2 = TestBed.createComponent(SettingsV2Component);
-    const component2 = fixture2.componentInstance;
-    fixture2.detectChanges();
-    await fixture2.whenStable();
-
-    const value2 = await firstValueFrom(component2["isBrowserAutofillSettingOverridden$"]);
-    expect(value2).toBe(false);
-  });
-
-  it("showAutofillBadge$ emits true when default autofill is NOT disabled and nudge is true", async () => {
+  it("showAutofillBadge$ emits true when showNudgeBadge is true", async () => {
     pushActiveAccount();
 
     mockNudges.showNudgeBadge$.mockImplementation((type: NudgeType) =>
@@ -184,30 +160,10 @@ describe("SettingsV2Component", () => {
     fixture.detectChanges();
     await fixture.whenStable();
 
-    mockAutofillSettings.defaultBrowserAutofillDisabled$.next(false);
-
     const value = await firstValueFrom(component.showAutofillBadge$);
     expect(value).toBe(true);
   });
 
-  it("showAutofillBadge$ emits false when default autofill IS disabled even if nudge is true", async () => {
-    pushActiveAccount();
-
-    mockNudges.showNudgeBadge$.mockImplementation((type: NudgeType) =>
-      of(type === NudgeType.AutofillNudge),
-    );
-
-    const fixture = TestBed.createComponent(SettingsV2Component);
-    const component = fixture.componentInstance;
-    fixture.detectChanges();
-    await fixture.whenStable();
-
-    mockAutofillSettings.defaultBrowserAutofillDisabled$.next(true);
-
-    const value = await firstValueFrom(component.showAutofillBadge$);
-    expect(value).toBe(false);
-  });
-
   it("dismissBadge dismisses when showVaultBadge$ emits true", async () => {
     const acct = pushActiveAccount();
 

apps/browser/src/tools/popup/settings/settings-v2.component.ts

@@ -1,16 +1,7 @@
 import { CommonModule } from "@angular/common";
 import { ChangeDetectionStrategy, Component } from "@angular/core";
 import { RouterModule } from "@angular/router";
-import {
-  combineLatest,
-  filter,
-  firstValueFrom,
-  from,
-  map,
-  Observable,
-  shareReplay,
-  switchMap,
-} from "rxjs";
+import { filter, firstValueFrom, Observable, shareReplay, switchMap } from "rxjs";
 
 import { PremiumUpgradeDialogComponent } from "@bitwarden/angular/billing/components";
 import { JslibModule } from "@bitwarden/angular/jslib.module";
@@ -28,8 +19,6 @@ import {
 } from "@bitwarden/components";
 
 import { CurrentAccountComponent } from "../../../auth/popup/account-switching/current-account.component";
-import { AutofillBrowserSettingsService } from "../../../autofill/services/autofill-browser-settings.service";
-import { BrowserApi } from "../../../platform/browser/browser-api";
 import { PopOutComponent } from "../../../platform/popup/components/pop-out.component";
 import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component";
 import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.component";
@@ -55,12 +44,6 @@ import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.co
 export class SettingsV2Component {
   NudgeType = NudgeType;
 
-  protected isBrowserAutofillSettingOverridden$ = from(
-    this.autofillBrowserSettingsService.isBrowserAutofillSettingOverridden(
-      BrowserApi.getBrowserClientVendor(window),
-    ),
-  );
-
   private authenticatedAccount$: Observable<Account> = this.accountService.activeAccount$.pipe(
     filter((account): account is Account => account !== null),
     shareReplay({ bufferSize: 1, refCount: true }),
@@ -82,23 +65,13 @@ export class SettingsV2Component {
     ),
   );
 
-  showAutofillBadge$: Observable<boolean> = combineLatest([
-    this.autofillBrowserSettingsService.defaultBrowserAutofillDisabled$,
-    this.authenticatedAccount$,
-  ]).pipe(
-    switchMap(([defaultBrowserAutofillDisabled, account]) =>
-      this.nudgesService.showNudgeBadge$(NudgeType.AutofillNudge, account.id).pipe(
-        map((badgeStatus) => {
-          return !defaultBrowserAutofillDisabled && badgeStatus;
-        }),
-      ),
-    ),
+  showAutofillBadge$: Observable<boolean> = this.authenticatedAccount$.pipe(
+    switchMap((account) => this.nudgesService.showNudgeBadge$(NudgeType.AutofillNudge, account.id)),
   );
 
   constructor(
     private readonly nudgesService: NudgesService,
     private readonly accountService: AccountService,
-    private readonly autofillBrowserSettingsService: AutofillBrowserSettingsService,
     private readonly accountProfileStateService: BillingAccountProfileStateService,
     private readonly dialogService: DialogService,
   ) {}

apps/browser/src/vault/popup/services/browser-autofill-nudge.service.spec.ts

@@ -0,0 +1,157 @@
+import { TestBed } from "@angular/core/testing";
+import { mock, MockProxy } from "jest-mock-extended";
+import { firstValueFrom } from "rxjs";
+
+import { NudgeStatus, NudgeType } from "@bitwarden/angular/vault";
+import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service";
+import { BrowserClientVendors } from "@bitwarden/common/autofill/constants";
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { StateProvider } from "@bitwarden/common/platform/state";
+import { UserId } from "@bitwarden/common/types/guid";
+
+import { FakeStateProvider, mockAccountServiceWith } from "../../../../../../libs/common/spec";
+import { BrowserApi } from "../../../platform/browser/browser-api";
+
+import { BrowserAutofillNudgeService } from "./browser-autofill-nudge.service";
+
+describe("BrowserAutofillNudgeService", () => {
+  let service: BrowserAutofillNudgeService;
+  let vaultProfileService: MockProxy<VaultProfileService>;
+  let fakeStateProvider: FakeStateProvider;
+
+  const userId = "test-user-id" as UserId;
+  const nudgeType = NudgeType.AutofillNudge;
+
+  const notDismissedStatus: NudgeStatus = {
+    hasBadgeDismissed: false,
+    hasSpotlightDismissed: false,
+  };
+
+  const dismissedStatus: NudgeStatus = {
+    hasBadgeDismissed: true,
+    hasSpotlightDismissed: true,
+  };
+
+  // Set profile creation date to now (new account, within 30 days)
+  const recentProfileDate = new Date();
+
+  beforeEach(() => {
+    vaultProfileService = mock<VaultProfileService>();
+    vaultProfileService.getProfileCreationDate.mockResolvedValue(recentProfileDate);
+
+    fakeStateProvider = new FakeStateProvider(mockAccountServiceWith(userId));
+
+    TestBed.configureTestingModule({
+      providers: [
+        BrowserAutofillNudgeService,
+        {
+          provide: VaultProfileService,
+          useValue: vaultProfileService,
+        },
+        {
+          provide: StateProvider,
+          useValue: fakeStateProvider,
+        },
+        {
+          provide: LogService,
+          useValue: mock<LogService>(),
+        },
+      ],
+    });
+
+    service = TestBed.inject(BrowserAutofillNudgeService);
+  });
+
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  describe("nudgeStatus$", () => {
+    it("returns parent status when browser client is Unknown", async () => {
+      jest
+        .spyOn(BrowserApi, "getBrowserClientVendor")
+        .mockReturnValue(BrowserClientVendors.Unknown);
+      jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(true);
+
+      const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId));
+
+      expect(result).toEqual(notDismissedStatus);
+    });
+
+    it("returns parent status when browser autofill is not overridden", async () => {
+      jest.spyOn(BrowserApi, "getBrowserClientVendor").mockReturnValue(BrowserClientVendors.Chrome);
+      jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(false);
+
+      const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId));
+
+      expect(result).toEqual(notDismissedStatus);
+    });
+
+    it("returns dismissed status when browser autofill is overridden", async () => {
+      jest.spyOn(BrowserApi, "getBrowserClientVendor").mockReturnValue(BrowserClientVendors.Chrome);
+      jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(true);
+
+      const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId));
+
+      expect(result).toEqual(dismissedStatus);
+    });
+
+    it("preserves parent dismissed status when account is older than 30 days", async () => {
+      // Set profile creation date to more than 30 days ago
+      const oldProfileDate = new Date(Date.now() - 31 * 24 * 60 * 60 * 1000);
+      vaultProfileService.getProfileCreationDate.mockResolvedValue(oldProfileDate);
+
+      jest.spyOn(BrowserApi, "getBrowserClientVendor").mockReturnValue(BrowserClientVendors.Chrome);
+      jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(false);
+
+      const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId));
+
+      expect(result).toEqual(dismissedStatus);
+    });
+
+    it("combines parent dismissed and browser autofill overridden status", async () => {
+      // Set profile creation date to more than 30 days ago (parent dismisses)
+      const oldProfileDate = new Date(Date.now() - 31 * 24 * 60 * 60 * 1000);
+      vaultProfileService.getProfileCreationDate.mockResolvedValue(oldProfileDate);
+
+      jest.spyOn(BrowserApi, "getBrowserClientVendor").mockReturnValue(BrowserClientVendors.Chrome);
+      jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(true);
+
+      const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId));
+
+      expect(result).toEqual(dismissedStatus);
+    });
+
+    it.each([
+      BrowserClientVendors.Chrome,
+      BrowserClientVendors.Edge,
+      BrowserClientVendors.Opera,
+      BrowserClientVendors.Vivaldi,
+    ])("checks browser autofill settings for %s browser", async (browserVendor) => {
+      const getBrowserClientVendorSpy = jest
+        .spyOn(BrowserApi, "getBrowserClientVendor")
+        .mockReturnValue(browserVendor);
+      const browserAutofillSettingsOverriddenSpy = jest
+        .spyOn(BrowserApi, "browserAutofillSettingsOverridden")
+        .mockResolvedValue(true);
+
+      await firstValueFrom(service.nudgeStatus$(nudgeType, userId));
+
+      expect(getBrowserClientVendorSpy).toHaveBeenCalledWith(window);
+      expect(browserAutofillSettingsOverriddenSpy).toHaveBeenCalled();
+    });
+
+    it("does not check browser autofill settings for Unknown browser", async () => {
+      jest
+        .spyOn(BrowserApi, "getBrowserClientVendor")
+        .mockReturnValue(BrowserClientVendors.Unknown);
+      const browserAutofillSettingsOverriddenSpy = jest
+        .spyOn(BrowserApi, "browserAutofillSettingsOverridden")
+        .mockResolvedValue(true);
+
+      await firstValueFrom(service.nudgeStatus$(nudgeType, userId));
+
+      expect(browserAutofillSettingsOverriddenSpy).not.toHaveBeenCalled();
+    });
+  });
+});

apps/browser/src/vault/popup/services/browser-autofill-nudge.service.ts

@@ -0,0 +1,37 @@
+import { Injectable } from "@angular/core";
+import { Observable, switchMap } from "rxjs";
+
+import { NudgeStatus, NudgeType } from "@bitwarden/angular/vault";
+import { NewAccountNudgeService } from "@bitwarden/angular/vault/services/custom-nudges-services/new-account-nudge.service";
+import { BrowserClientVendors } from "@bitwarden/common/autofill/constants";
+import { UserId } from "@bitwarden/common/types/guid";
+
+import { BrowserApi } from "../../../platform/browser/browser-api";
+
+/**
+ * Browser-specific autofill nudge service.
+ * Extends NewAccountNudgeService (30-day account age check) and adds
+ * browser autofill setting detection.
+ *
+ * Nudge is dismissed if:
+ * - Account is older than 30 days (inherited from NewAccountNudgeService)
+ * - Browser's built-in password manager is already disabled via privacy settings
+ */
+@Injectable()
+export class BrowserAutofillNudgeService extends NewAccountNudgeService {
+  override nudgeStatus$(nudgeType: NudgeType, userId: UserId): Observable<NudgeStatus> {
+    return super.nudgeStatus$(nudgeType, userId).pipe(
+      switchMap(async (status) => {
+        const browserClient = BrowserApi.getBrowserClientVendor(window);
+        const browserAutofillOverridden =
+          browserClient !== BrowserClientVendors.Unknown &&
+          (await BrowserApi.browserAutofillSettingsOverridden());
+
+        return {
+          hasBadgeDismissed: status.hasBadgeDismissed || browserAutofillOverridden,
+          hasSpotlightDismissed: status.hasSpotlightDismissed || browserAutofillOverridden,
+        };
+      }),
+    );
+  }
+}

libs/angular/src/vault/index.ts

@@ -1,3 +1,4 @@
 // Note: Nudge related code is exported from `libs/angular` because it is consumed by multiple
 // `libs/*` packages. Exporting from the `libs/vault` package creates circular dependencies.
 export { NudgesService, NudgeStatus, NudgeType } from "./services/nudges.service";
+export { AUTOFILL_NUDGE_SERVICE } from "./services/nudge-injection-tokens";

libs/angular/src/vault/services/custom-nudges-services/index.ts

@@ -4,3 +4,4 @@ export * from "./empty-vault-nudge.service";
 export * from "./vault-settings-import-nudge.service";
 export * from "./new-item-nudge.service";
 export * from "./new-account-nudge.service";
+export * from "./noop-nudge.service";

libs/angular/src/vault/services/custom-nudges-services/noop-nudge.service.ts

@@ -0,0 +1,27 @@
+import { Injectable } from "@angular/core";
+import { Observable, of } from "rxjs";
+
+import { UserId } from "@bitwarden/common/types/guid";
+
+import { SingleNudgeService } from "../default-single-nudge.service";
+import { NudgeStatus, NudgeType } from "../nudges.service";
+
+/**
+ * A no-op nudge service that always returns dismissed status.
+ * Use this for nudges that should be completely ignored/hidden in certain clients.
+ * For example, browser-specific nudges can use this as the default in non-browser clients.
+ */
+@Injectable({ providedIn: "root" })
+export class NoOpNudgeService implements SingleNudgeService {
+  nudgeStatus$(_nudgeType: NudgeType, _userId: UserId): Observable<NudgeStatus> {
+    return of({ hasBadgeDismissed: true, hasSpotlightDismissed: true });
+  }
+
+  async setNudgeStatus(
+    _nudgeType: NudgeType,
+    _newStatus: NudgeStatus,
+    _userId: UserId,
+  ): Promise<void> {
+    // No-op: state changes are ignored
+  }
+}

libs/angular/src/vault/services/nudge-injection-tokens.ts

@@ -0,0 +1,7 @@
+import { InjectionToken } from "@angular/core";
+
+import { SingleNudgeService } from "./default-single-nudge.service";
+
+export const AUTOFILL_NUDGE_SERVICE = new InjectionToken<SingleNudgeService>(
+  "AutofillNudgeService",
+);

libs/angular/src/vault/services/nudges.service.ts

@@ -12,8 +12,10 @@ import {
   NewItemNudgeService,
   AccountSecurityNudgeService,
   VaultSettingsImportNudgeService,
+  NoOpNudgeService,
 } from "./custom-nudges-services";
 import { DefaultSingleNudgeService, SingleNudgeService } from "./default-single-nudge.service";
+import { AUTOFILL_NUDGE_SERVICE } from "./nudge-injection-tokens";
 
 export type NudgeStatus = {
   hasBadgeDismissed: boolean;
@@ -56,6 +58,12 @@ export class NudgesService {
   private newItemNudgeService = inject(NewItemNudgeService);
   private newAcctNudgeService = inject(NewAccountNudgeService);
 
+  // NoOp service that always returns dismissed
+  private noOpNudgeService = inject(NoOpNudgeService);
+
+  // Optional Browser-specific service provided via injection token (not all clients have autofill)
+  private autofillNudgeService = inject(AUTOFILL_NUDGE_SERVICE, { optional: true });
+
   /**
    * Custom nudge services to use for specific nudge types
    * Each nudge type can have its own service to determine when to show the nudge
@@ -66,7 +74,7 @@ export class NudgesService {
     [NudgeType.EmptyVaultNudge]: inject(EmptyVaultNudgeService),
     [NudgeType.VaultSettingsImportNudge]: inject(VaultSettingsImportNudgeService),
     [NudgeType.AccountSecurity]: inject(AccountSecurityNudgeService),
-    [NudgeType.AutofillNudge]: this.newAcctNudgeService,
+    [NudgeType.AutofillNudge]: this.autofillNudgeService ?? this.noOpNudgeService,
     [NudgeType.DownloadBitwarden]: this.newAcctNudgeService,
     [NudgeType.GeneratorNudgeStatus]: this.newAcctNudgeService,
     [NudgeType.NewLoginItemStatus]: this.newItemNudgeService,