PM-25654: feat: Preview attachment

[david-livefront]

🎟️ Tracking

PM-25654
Community Forum

📔 Objective

This PR adds the Preview Attachment functionality to the VaultItemScreen.

Viewing image attachments is a hassle since you have to download, save, and open them in another app.
This adds a built-in preview for images, Ideally, we’d have a full file manager that supports all types, but since images are the ones most often needing quick viewing, I think this is okay for now.

The implementation:

• Downloads the file temporarily.
• Loads and displays the image with zoom and pan support.
• Automatically deletes the file once it has been loaded.

📸 Screenshots

Screen_recording_20260316_093623.mp4

[claude]

Claude finished @david-livefront's task in 3m 37s —— View job

---

Overall Assessment: APPROVE

This PR adds an image attachment preview feature, allowing users to tap vault item attachments to preview supported image formats (PNG, JPG, JPEG, GIF, WEBP, BMP) in a dedicated screen with pinch-to-zoom and pan gestures. Non-previewable file types display an informative error with a download option. The implementation introduces a new PreviewAttachmentViewModel, PreviewAttachmentScreen, and reusable ImagePreviewContent composable with bitmap downsampling. Navigation is wired through all vault item content types (login, card, identity, secure note, SSH key) and the BitwardenBasicDialog gains a customizable confirm button label.

Code Review Details

No new findings. Previous review findings have been addressed by the developer. The one unresolved inline thread from SaintPatrck regarding adding errorMessage to DownloadAttachmentResult.Failure for Cookie Redirect Exception handling is already being tracked.

[codecov]

Codecov Report

❌ Patch coverage is 82.49400% with 73 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.73%. Comparing base (6d04c04) to head (a44ba4f).

Files with missing lines	Patch %	Lines
.../attachments/preview/PreviewAttachmentViewModel.kt	91.76%	15 Missing and 5 partials ⚠️
...ure/attachments/preview/PreviewAttachmentScreen.kt	76.54%	15 Missing and 4 partials ⚠️
...attachments/preview/PreviewAttachmentNavigation.kt	21.73%	18 Missing ⚠️
...warden/ui/vault/feature/item/VaultItemViewModel.kt	35.71%	8 Missing and 1 partial ⚠️
...i/vault/feature/item/VaultItemAttachmentContent.kt	88.88%	5 Missing ⚠️
...arden/ui/vault/feature/item/VaultItemNavigation.kt	0.00%	1 Missing ⚠️
...ature/item/handlers/VaultCommonItemTypeHandlers.kt	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6675      +/-   ##
==========================================
- Coverage   85.80%   85.73%   -0.07%     
==========================================
  Files         807      810       +3     
  Lines       57331    57738     +407     
  Branches     8362     8417      +55     
==========================================
+ Hits        49193    49502     +309     
- Misses       5220     5307      +87     
- Partials     2918     2929      +11     

Flag	Coverage Δ
app-data	17.55% <0.00%> (-0.12%)	⬇️
app-ui-auth-tools	20.66% <0.00%> (-0.14%)	⬇️
app-ui-platform	14.96% <0.24%> (-0.10%)	⬇️
app-ui-vault	26.21% <82.25%> (+0.37%)	⬆️
authenticator	6.57% <0.00%> (-0.03%)	⬇️
lib-core-network-bridge	4.26% <0.00%> (-0.03%)	⬇️
lib-data-ui	0.95% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Checkmarx One – Scan Summary & Details – 9c17c508-d0d5-455b-a366-a42b0464f0a3

---

New Issues (1)

Checkmarx found the following issues in this Pull Request

#	Severity	Issue	Source File / Package	Checkmarx Insight
1		Use_of_Hardcoded_Password	/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt: 147	details The application uses the hard-coded password "cipherId1234" for authentication purposes, either using it to verify users' identities, or to access... Attack Vector

[claude]

test read access

[claude]

Overall Assessment: REQUEST CHANGES

Reviewed the new Preview Attachment feature across 21 changed files (1965 additions, 51 deletions). The architecture follows established Bitwarden patterns well (BaseViewModel UDF, navigation, screen/VM separation). Two issues need to be addressed before merge.

Code Review Details

• ❌ CRITICAL: Bitmap render error shows "Delete" to the user instead of a meaningful error message

  • PreviewAttachmentViewModel.kt:329-336 — handleBitmapRenderError() uses BitwardenString.delete (which resolves to the string "Delete") as the error message when an image fails to render. This PR added the string resource bitwarden_could_not_decrypt_this_file_so_the_preview_cannot_be_displayed in strings.xml:1231 specifically for this use case, but it is never referenced in any Kotlin code. The fix is to replace BitwardenString.delete.asText() with BitwardenString.bitwarden_could_not_decrypt_this_file_so_the_preview_cannot_be_displayed.asText() in handleBitmapRenderError(), and update the corresponding assertion in PreviewAttachmentViewModelTest.kt at line 1529.

• ⚠️ IMPORTANT: .first() can suspend indefinitely if cipher data is never non-null, leaving the user with a stuck loading dialog

  • PreviewAttachmentViewModel.kt:310-321 — In handleDownloadClick(), the expression .mapNotNull { it.data }.first() will never complete if the vault item state flow only emits states with null data (e.g., persistent DataState.Error(data = null)). The user sees a loading dialog with no way to dismiss it. The existing VaultItemViewModel.handleAttachmentDownloadClick avoids this pattern by using requireNotNull(content.common.currentCipher) from already-loaded state rather than creating a new flow subscription. Consider adding a timeout via withTimeoutOrNull or restructuring to use already-available cipher data.

[SaintPatrck]

Can we go ahead and add errorMessage to DownloadAttachmentResult.Failure to handle the potential Cookie Redirect Exception?

[SaintPatrck]

Requesting hold until after next RC cut.