[PM-25083] - Autofill confirmation dialog

[jaasen-livefront]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-26650

📔 Objective

This PR adds a new UI flow for the "Fill and Save" function to include a new dialog to confirm autofill function.

📸 Screenshots

Screen.Recording.2025-10-10.at.12.36.03.PM.mov

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[claude]

Claude finished @jaasen-livefront's task —— View job

---

Code Review Complete

Summary of Changes

This PR adds a confirmation dialog for the "Fill and Save" autofill function, warning users when a site doesn't match saved login details. The implementation includes:

• New AutofillConfirmationDialogComponent with three action options
• Integration into ItemMoreOptionsComponent.doAutofillAndSave()
• Feature flag AutofillConfirmation to control the feature
• i18n strings for the dialog
• Minor callout component HTML fix

---

❌ Critical Issues

1. Inverted Feature Flag Logic (apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts:192)

if (!isFeatureFlagEnabled) {
  // Shows dialog when flag is FALSE

Problem: The dialog is shown when the feature flag is disabled (!isFeatureFlagEnabled), which is backwards. This means:

• When the flag is false (default), the dialog shows
• When the flag is true (enabled), the dialog is hidden
• This defeats the purpose of the feature flag

Fix: Change to if (isFeatureFlagEnabled) {

2. Hardcoded URL in Template (apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.html:11)

<bit-callout type="warning" icon="bwi-globe" class="tw-mb-2 tw-truncate"
  >somereallylongurlthatneedstobeshortened.com</bit-callout
>

Problem: The actual currentUri parameter is passed to the component but never used. The template displays a hardcoded placeholder string instead.

Fix: Use {{ currentUri }} in the template to display the actual URI.

3. TODO Left in Production Code (autofill-confirmation-dialog.component.html:19)

href="TODO: Add phishing link"

Problem: Production code should not contain TODO comments, especially in user-facing elements.

Fix: Either add the actual phishing documentation URL or remove the link temporarily.

4. Unused Parameter (autofill-confirmation-dialog.component.ts:18, 39)

savedUris?: string[];  // Defined in params
savedUris: string[] = [];  // Initialized but never used

Problem: The savedUris parameter is defined in the interface and initialized in the component but never used anywhere. This suggests incomplete implementation or unnecessary code.

Fix: Either implement the functionality that uses savedUris or remove it from the interface and component.

---

🎨 Suggested Improvements

5. Missing Test Coverage

The Codecov report shows 0% coverage for 35 new lines. Both new files lack unit tests:

• autofill-confirmation-dialog.component.ts (20 lines missing)
• item-more-options.component.ts (15 lines missing)

Recommendation: Add unit tests covering:

• Dialog opening with correct parameters
• All three button actions (add URI, autofill only, cancel)
• Feature flag conditional logic (once fixed)
• Edge cases (null/undefined URIs)

6. Violation of ADR-0025: TypeScript Enum Usage (libs/common/src/enums/feature-flag.enum.ts:12)

export enum FeatureFlag {

Issue: The project has standardized on not using TypeScript enums (see CLAUDE.md ADR-0025). This file already has a FIXME comment acknowledging this, but new entries continue to be added.

Note: While this is a pre-existing issue, it's worth noting that adding to a known anti-pattern increases technical debt. Consider prioritizing the conversion to const objects.

7. Accessibility: Empty Callout Title (autofill-confirmation-dialog.component.html:10)

<bit-callout type="warning" ...>

Issue: The callout has an empty title attribute, which may affect screen reader announcements.

Recommendation: Either provide a meaningful title (e.g., "Current website") or verify that the component handles empty titles accessibly.

8. Magic Numbers in Static Method (autofill-confirmation-dialog.component.ts:60-68)

static open(
  dialogService: DialogService,
  config: DialogConfig<AutofillConfirmationDialogParams>,
) {
  return dialogService.open<AutofillConfirmationDialogResultType>(
    AutofillConfirmationDialogComponent,
    { ...config },
  );
}

Issue: The static open method spreads the config without any validation or default values. While this pattern is common, it provides no additional value over calling dialogService.open directly.

Recommendation: Either add meaningful defaults/validation to justify the helper method, or consider removing it and calling dialogService.open directly where needed.

9. Inconsistent Component Initialization (autofill-confirmation-dialog.component.ts:44-46)

constructor(
  @Inject(DIALOG_DATA) protected params: AutofillConfirmationDialogParams,
  private dialogRef: DialogRef,
) {
  this.currentUri = params.currentUri;
}

Issue: Only currentUri is extracted in the constructor, while savedUris is initialized to an empty array. This asymmetry suggests the component design is incomplete.

Recommendation: Either use both parameters consistently or remove savedUris entirely.

10. Missing Type Safety for Tab URL (item-more-options.component.ts:193-194)

const currentTab = await firstValueFrom(this.vaultPopupAutofillService.currentAutofillTab$);
const currentUri = Utils.getHostname(currentTab.url);

Issue: No null check for currentTab or currentTab.url before calling Utils.getHostname(). If the tab or URL is undefined, this could throw an error.

Recommendation: Add defensive checks:

if (!currentTab?.url) {
  // Handle error case
  return;
}

---

👍 Good Practices Observed

• Proper use of Angular's standalone component pattern with explicit imports
• Feature flag integration following project patterns
• i18n implementation for all user-facing strings
• Arrow function methods to avoid binding issues
• Type-safe dialog result using const object pattern (ADR-0025 compliant for the dialog result)

---

📋 Action Items for Author

Must Fix Before Merge:

1. ❌ Fix inverted feature flag logic (item-more-options.component.ts:192)
2. ❌ Display actual URI instead of hardcoded string (autofill-confirmation-dialog.component.html:11)
3. ❌ Remove or resolve TODO comment (autofill-confirmation-dialog.component.html:19)

Should Address:
4. 🔧 Add unit tests for both new components
5. 🔧 Remove unused savedUris parameter or implement its functionality
6. 🔧 Add null checks for currentTab.url

Consider:
7. 💭 Review accessibility of empty callout title
8. 💭 Evaluate whether the static open helper method is necessary

---

[github-actions]

Checkmarx One – Scan Summary & Details – d5993afb-57ad-4d3c-989f-050a5766eb34

Great job! No new security vulnerabilities introduced in this pull request

[codecov]

Codecov Report

❌ Patch coverage is 0% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.91%. Comparing base (bc48164) to head (5c7cd47).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...n-dialog/autofill-confirmation-dialog.component.ts	0.00%	19 Missing ⚠️
...2/item-more-options/item-more-options.component.ts	0.00%	16 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #16835      +/-   ##
==========================================
+ Coverage   38.87%   38.91%   +0.03%     
==========================================
  Files        3420     3436      +16     
  Lines       97313    97518     +205     
  Branches    14625    14663      +38     
==========================================
+ Hits        37830    37947     +117     
- Misses      57827    57912      +85     
- Partials     1656     1659       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud