PM-28324: Add a guard that conditionally forces a popout depending on platform

[abergs]

🎟️ Tracking

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

📔 Objective

This PR adds a new route guard that can force a browser extension popout for certain platforms. We then use that guard to solve a window focus problem for login with passkeys on linux.

To test on another platform than linux, please add , true to the route arguments for force a popout

📸 Screenshots

https://share.cleanshot.com/86L4zpHd

⏰ 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 @abergs's task —— View job

---

Code Review Summary

This PR adds a route guard to force browser extension popouts on specific platforms, solving a window focus issue with passkey login on Linux. The implementation is clean and well-tested.

---

Findings

Finding 1: PR title and description need improvement

The PR title lacks specificity about the problem being solved. Consider:

• Current: "Add a guard that conditionally forces a popout depending on platform"
• Suggested: "Fix Linux passkey login focus issue by forcing popout window"

Objective section improvements

The objective should better explain the root problem:

## 📔 Objective

Fixes a window focus issue on Linux where the browser extension popup closes during WebAuthn/passkey authentication, interrupting the login flow. 

This PR introduces a new `platformPopoutGuard` that forces the extension to open in a popout window (instead of the regular popup) on affected platforms before navigating to the passkey login route. After successful authentication, the popout automatically closes and reopens the regular popup.

The guard is platform-configurable and includes a `forcePopout` parameter for cross-platform testing.

Finding 2: Test suite is comprehensive and well-structured

The test suite (platform-popout.guard.spec.ts) provides excellent coverage:

• ✅ Platform matching logic
• ✅ Popout/sidebar detection
• ✅ Force popout override
• ✅ Multiple platform support
• ✅ URL and query parameter handling
• ✅ Proper use of mockImplementation() instead of mockReturnValue() for async methods

Finding 3: Integration with LoginViaWebAuthnComponent is clean

The auto-close behavior (lines 136-143 in login-via-webauthn.component.ts:136-143) properly:

• Checks query parameter for auto-close flag
• Only applies to browser extensions
• Reopens regular popup before closing popout
• Returns early to avoid navigation

Finding 4: Comparison with FilePopoutUtilsService approach

The comment from @djsmith85 mentions FilePopoutUtilsService as an alternative. Key differences:

Aspect	platformPopoutGuard	FilePopoutUtilsService
Approach	Automatic (guard blocks navigation)	Manual (shows callout, user clicks button)
User action	None required	User must click to popout
Use case	Time-sensitive operations (passkey auth)	File operations (import/send)
Platform detection	Uses BrowserApi.getPlatformInfo()	Uses user agent strings

For passkey authentication, the automatic guard approach is more appropriate because:

• WebAuthn has timing constraints
• User shouldn't need to understand why a popout is needed
• The auth flow would be interrupted by showing a callout

Finding 5: Missing test coverage is in routing configuration

Codecov reports 1 line missing coverage at apps/browser/src/popup/app-routing.module.ts:418 - this is the canActivate array with the guard. This is acceptable as:

• Routing configuration is typically not unit tested
• The guard itself has 100% coverage
• Integration/E2E tests would verify routing behavior

---

Recommendations

1. 💭 Consider documenting the UX difference from FilePopoutUtilsService: Since there are now two popout patterns in the codebase, it would be helpful to document when to use each approach (perhaps in comments or ADR).

2. 🎨 Minor: Guard could be more defensive with params.toString(): If there are no query params and no autoClosePopout, the URL will have a trailing ?. Consider:

   const urlWithAutoClose = params.size > 0 ? `${path}?${params.toString()}` : path;

   However, this is very minor as Angular handles trailing ? gracefully.

3. 💭 Question: Should other platforms be included?: The PR description mentions testing with , true to force popout on other platforms. Have you tested on macOS/Windows to confirm they don't have the same focus issue? The FilePopoutUtilsService suggests macOS has similar issues with file operations.

---

Positive Observations

• Guard implementation is clean and follows Angular best practices
• Functional style with CanActivateFn (not class-based)
• Excellent JSDoc documentation
• Test suite is thorough with good describe organization
• Proper separation of concerns between guard and component
• autoClosePopout pattern maintains good UX by not leaving the popout open

---

Overall Assessment: Solid implementation that solves a real UX problem on Linux. Tests were added as requested by @rr-bw. The automatic guard approach is more appropriate than a manual callout for time-sensitive auth operations.

[github-actions]

Checkmarx One – Scan Summary & Details – 776e320e-6877-420a-827a-eefb6a63d1f2

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

[codecov]

Codecov Report

❌ Patch coverage is 60.00000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.26%. Comparing base (9e6d0cc) to head (abc991a).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...login-via-webauthn/login-via-webauthn.component.ts	0.00%	11 Missing ⚠️
apps/browser/src/popup/app-routing.module.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #17444      +/-   ##
==========================================
+ Coverage   41.23%   41.26%   +0.02%     
==========================================
  Files        3543     3547       +4     
  Lines      101963   102069     +106     
  Branches    15295    15313      +18     
==========================================
+ Hits        42048    42117      +69     
- Misses      58152    58187      +35     
- Partials     1763     1765       +2     

☔ 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.

[rr-bw]

Looks great, would you mind adding a small test suite?

[djsmith85]

@abergs / @bitwarden/team-tools-dev An alternative approach to the FilePopoutUtilsServicewhich is used to control popping out the extension window for file-uploads on the Import and Sends (file) page

[Hinton]

It seems the main difference is that in tool related code we show a prompt to the user rather than automatically opening the popout.