PM-22836 Import popout crashes unexpectedly

[harr1424]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-22836?atlOrigin=eyJpIjoiNTUwZTI2NmZiMzRhNDIzNTlmMTAzMGJkZWZjZDhjOGMiLCJwIjoiaiJ9

📔 Objective

• Firefox has a known issue that causes the Bitwarden extension to crash when a file picker (NSOpenPanel on Mac OS) is opened outside of a popp-out context. • The Bitwarden extension exposes a file picker in at least two situations: when creating a file type send and when importing credentials into a vault. • To avoid the crash it is necessary to enforce the invariant that a view exposing a file picker can only ever appear in a popped-out context. • The UI logic specific to file type sends already prompted the user to pop-out the extension, but it didn't prevent the user from closing the pop-out and re-opening the extension in a not-popped-out context and then attempting to open a file picker. • It was determined in an internal Slack discussion that the pop-out will be automatic (for consistency) and not require user confirmation. • This PR implements the invariant that the views exposing a file picker for both file type sends and vault imports are only ever accessible in a pop-out, and per the conversation linked above, the pop-out occurs without user intervention/confirmation.

📸 Screenshots

⏰ 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

[github-actions[bot]]

Checkmarx One – Scan Summary & Details – 6fa5dc74-037c-47ce-8f7a-f2dd0bf657c6

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

[codecov[bot]]

Codecov Report

:x: Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review. :white_check_mark: Project coverage is 41.85%. Comparing base (dc763f6) to head (beb79c8). :warning: Report is 62 commits behind head on main. :white_check_mark: All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
apps/browser/src/popup/app-routing.module.ts	0.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #16928      +/-   ##
==========================================
+ Coverage   41.84%   41.85%   +0.01%     
==========================================
  Files        3589     3590       +1     
  Lines      104162   104207      +45     
  Branches    15713    15720       +7     
==========================================
+ Hits        43590    43621      +31     
- Misses      58722    58731       +9     
- Partials     1850     1855       +5     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

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

[sonarqubecloud[bot]]

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

[claude[bot]]

Claude finished @harr1424's task —— View job

---

Code Review Complete ✅

The bitwarden-code-reviewer agent has completed its analysis and posted detailed inline comments on the PR.

Review Summary

Overall Assessment: High-quality implementation with excellent test coverage, but one critical route is missing protection.

Strengths:

• ✅ Well-architected guard solution that correctly detects Firefox and popout context
• ✅ Comprehensive test suite (194 lines) covering all scenarios
• ✅ Proper URL preservation including query parameters
• ✅ Cross-browser safety (only affects Firefox)
• ✅ Good documentation with Firefox bug reference

Critical Issue:

• ⚠️ Missing guard on /attachments route - The attachments route also uses a file picker but doesn't have the firefoxPopoutGuard() applied, which will cause crashes when users upload attachments on Firefox.

Recommendation: Add the guard to the attachments route before merging. The implementation is otherwise solid and ready.

See the inline comments on the PR for specific details and code references.

---

[Hinton]

@harr1424 This PR seems to do several things, what parts of this actually resolves the stated issue, i.e. popout crashes.

[harr1424]

@harr1424 This PR seems to do several things, what parts of this actually resolves the stated issue, i.e. popout crashes.

@Hinton Depending upon user behavior the following file changes are required to prevent a pop-out crash on Firefox. See the root cause of the problem this PR defends against.

• apps/browser/src/popup/app-routing.module.ts
• apps/browser/src/tools/popup/send-v2/send-file-popout-dialog/send-file-popout-dialog.component.ts
• apps/browser/src/_locales/en/messages.json
• apps/browser/src/tools/popup/services/file-popout-utils.service.ts
• apps/browser/src/tools/popup/settings/import/import-browser-v2.component.html
• apps/browser/src/tools/popup/settings/import/import-browser-v2.component.ts
• apps/browser/src/tools/popup/settings/import/import-file-popout-dialog-container.component.ts
• apps/browser/src/tools/popup/settings/import/import-file-popout-dialog.component.html
• apps/browser/src/tools/popup/settings/import/import-file-popout-dialog.component.ts
• apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts

The following files don't directly address the Firefox issue, but were used to resolve a situation where the extension never loaded. @MGibson1 has since merged changes that resolves this on main. I can remove these files and re-test to make sure everything still works and then re-submit if this is your preference.

• apps/browser/src/platform/storage/foreground-memory-storage.service.ts (this would add some helpful logging in the event a breaking change similar to last week's occurs)
• apps/browser/src/popup/app.component.ts (this one is likely worth keeping)