From 7a70c98158929561004615e88b3d897621f53976 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Apr 2026 15:43:10 +0000 Subject: [PATCH 1/3] fix: image sharing fails due to coroutine scope cancellation The share media dialog used `rememberCoroutineScope()` which shadowed the outer `accountViewModel.viewModelScope`. When onDismiss() was called immediately after launching the share coroutine, the dialog-scoped coroutine was cancelled, causing CancellationException to be caught as a generic error and showing "Unable to share image" toast. Fix: use viewModelScope for share operations so they survive dialog dismissal, and re-throw CancellationException in shareImageFile. https://claude.ai/code/session_01Euj5mXfjneNCx9m49YTp92 --- .../amethyst/ui/components/ZoomableContentView.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt index 6321e4fd4..336d387db 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt @@ -762,7 +762,7 @@ fun ShareMediaAction( content: BaseMediaContent? = null, accountViewModel: AccountViewModel, ) { - val scope = accountViewModel.viewModelScope + val viewModelScope = accountViewModel.viewModelScope // Track if video is downloading - hoisted here to block menu dismiss during download val isDownloadingVideo = remember { mutableStateOf(false) } @@ -817,7 +817,7 @@ fun ShareMediaAction( videoUri?.let { if (videoUri.isNotEmpty()) { M3ActionRow(icon = Icons.Outlined.Share, text = stringRes(R.string.share_image)) { - scope.launch { shareImageFile(context, videoUri, mimeType) } + viewModelScope.launch { shareImageFile(context, videoUri, mimeType) } onDismiss() } } @@ -833,7 +833,7 @@ fun ShareMediaAction( enabled = !isDownloadingVideo.value, ) { isDownloadingVideo.value = true - scope.launch { + viewModelScope.launch { shareVideoFile( context = context, videoUrl = videoUri, @@ -858,7 +858,7 @@ fun ShareMediaAction( is MediaLocalVideo -> { content.localFile?.let { localFile -> M3ActionRow(icon = Icons.Outlined.Share, text = stringRes(R.string.share_video)) { - scope.launch { shareLocalVideoFile(context, localFile, mimeType) } + viewModelScope.launch { shareLocalVideoFile(context, localFile, mimeType) } onDismiss() } } @@ -894,6 +894,7 @@ private suspend fun shareImageFile( context.startActivity(Intent.createChooser(shareIntent, null)) } catch (e: Exception) { + if (e is CancellationException) throw e Log.w("ZoomableContentView", "Failed to share image: $videoUri", e) Toast.makeText(context, context.getString(R.string.unable_to_share_image), Toast.LENGTH_SHORT).show() } From 074cde0bd10f9e0bdce4e5bd41894e014fddbe3e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Apr 2026 15:52:21 +0000 Subject: [PATCH 2/3] test: add media extension detection tests for ShareHelper Make getImageExtension, getVideoExtension, and getMediaExtension internal for testing. Add tests covering all supported image formats (JPEG, PNG, GIF, WebP) and video formats (WebM, AVI, MP4, MOV), plus edge cases (unknown formats, empty files, short headers). https://claude.ai/code/session_01Euj5mXfjneNCx9m49YTp92 --- .../amethyst/ui/components/ShareHelper.kt | 6 +- .../amethyst/ui/components/ShareHelperTest.kt | 113 ++++++++++++++++++ 2 files changed, 116 insertions(+), 3 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ShareHelper.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ShareHelper.kt index 2b91c47c1..4be867651 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ShareHelper.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ShareHelper.kt @@ -85,11 +85,11 @@ object ShareHelper { } ?: throw IOException("Unable to open snapshot for: $imageUrl") } - private fun getImageExtension(file: File): String = getMediaExtension(file, isVideo = false) + internal fun getImageExtension(file: File): String = getMediaExtension(file, isVideo = false) - private fun getVideoExtension(file: File): String = getMediaExtension(file, isVideo = true) + internal fun getVideoExtension(file: File): String = getMediaExtension(file, isVideo = true) - private fun getMediaExtension( + internal fun getMediaExtension( file: File, isVideo: Boolean, ): String { diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/ShareHelperTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/ShareHelperTest.kt index 509e40670..8bbc8a29a 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/ShareHelperTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/ShareHelperTest.kt @@ -49,4 +49,117 @@ class ShareHelperTest { targetFile.delete() tempDir.delete() } + + @Test + fun getImageExtension_jpeg() { + val file = createTempFileWithBytes(byteArrayOf(0xFF.toByte(), 0xD8.toByte(), 0x00, 0x00)) + assertEquals("jpg", ShareHelper.getImageExtension(file)) + file.delete() + } + + @Test + fun getImageExtension_png() { + val file = createTempFileWithBytes(byteArrayOf(0x89.toByte(), 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A)) + assertEquals("png", ShareHelper.getImageExtension(file)) + file.delete() + } + + @Test + fun getImageExtension_gif() { + val file = createTempFileWithBytes("GIF89a".toByteArray()) + assertEquals("gif", ShareHelper.getImageExtension(file)) + file.delete() + } + + @Test + fun getImageExtension_webp() { + // RIFF....WEBP + val header = ByteArray(12) + "RIFF".toByteArray().copyInto(header, 0) + "WEBP".toByteArray().copyInto(header, 8) + val file = createTempFileWithBytes(header) + assertEquals("webp", ShareHelper.getImageExtension(file)) + file.delete() + } + + @Test + fun getImageExtension_unknownDefaultsToJpg() { + val file = createTempFileWithBytes(byteArrayOf(0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07)) + assertEquals("jpg", ShareHelper.getImageExtension(file)) + file.delete() + } + + @Test + fun getImageExtension_tooShortDefaultsToJpg() { + val file = createTempFileWithBytes(byteArrayOf(0x00, 0x01)) + assertEquals("jpg", ShareHelper.getImageExtension(file)) + file.delete() + } + + @Test + fun getVideoExtension_webm() { + val file = createTempFileWithBytes(byteArrayOf(0x1A, 0x45, 0xDF.toByte(), 0xA3.toByte(), 0x00, 0x00)) + assertEquals("webm", ShareHelper.getVideoExtension(file)) + file.delete() + } + + @Test + fun getVideoExtension_avi() { + // RIFF....AVI + val header = ByteArray(12) + "RIFF".toByteArray().copyInto(header, 0) + "AVI ".toByteArray().copyInto(header, 8) + val file = createTempFileWithBytes(header) + assertEquals("avi", ShareHelper.getVideoExtension(file)) + file.delete() + } + + @Test + fun getVideoExtension_mp4Isom() { + // ....ftypisom + val header = ByteArray(12) + "ftyp".toByteArray().copyInto(header, 4) + "isom".toByteArray().copyInto(header, 8) + val file = createTempFileWithBytes(header) + assertEquals("mp4", ShareHelper.getVideoExtension(file)) + file.delete() + } + + @Test + fun getVideoExtension_mov() { + // ....ftypqt__ + val header = ByteArray(12) + "ftyp".toByteArray().copyInto(header, 4) + "qt ".toByteArray().copyInto(header, 8) + val file = createTempFileWithBytes(header) + assertEquals("mov", ShareHelper.getVideoExtension(file)) + file.delete() + } + + @Test + fun getVideoExtension_unknownDefaultsToMp4() { + val file = createTempFileWithBytes(byteArrayOf(0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07)) + assertEquals("mp4", ShareHelper.getVideoExtension(file)) + file.delete() + } + + @Test + fun getMediaExtension_emptyFileDefaultsToImage() { + val file = createTempFileWithBytes(byteArrayOf()) + assertEquals("jpg", ShareHelper.getMediaExtension(file, isVideo = false)) + file.delete() + } + + @Test + fun getMediaExtension_emptyFileDefaultsToVideo() { + val file = createTempFileWithBytes(byteArrayOf()) + assertEquals("mp4", ShareHelper.getMediaExtension(file, isVideo = true)) + file.delete() + } + + private fun createTempFileWithBytes(bytes: ByteArray): File { + val file = File.createTempFile("sharehelpertest", ".tmp") + file.writeBytes(bytes) + return file + } } From c88c961488617f16efb203570f725d2c2ec3dba4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Apr 2026 15:56:21 +0000 Subject: [PATCH 3/3] refactor: simplify scope usage and fix test temp file leak - Remove unnecessary local alias; use accountViewModel.viewModelScope directly at call sites for clarity about scope ownership - Add deleteOnExit() to test temp files so they're cleaned up even if assertions fail https://claude.ai/code/session_01Euj5mXfjneNCx9m49YTp92 --- .../amethyst/ui/components/ZoomableContentView.kt | 8 +++----- .../amethyst/ui/components/ShareHelperTest.kt | 1 + 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt index 336d387db..df1559569 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt @@ -762,8 +762,6 @@ fun ShareMediaAction( content: BaseMediaContent? = null, accountViewModel: AccountViewModel, ) { - val viewModelScope = accountViewModel.viewModelScope - // Track if video is downloading - hoisted here to block menu dismiss during download val isDownloadingVideo = remember { mutableStateOf(false) } @@ -817,7 +815,7 @@ fun ShareMediaAction( videoUri?.let { if (videoUri.isNotEmpty()) { M3ActionRow(icon = Icons.Outlined.Share, text = stringRes(R.string.share_image)) { - viewModelScope.launch { shareImageFile(context, videoUri, mimeType) } + accountViewModel.viewModelScope.launch { shareImageFile(context, videoUri, mimeType) } onDismiss() } } @@ -833,7 +831,7 @@ fun ShareMediaAction( enabled = !isDownloadingVideo.value, ) { isDownloadingVideo.value = true - viewModelScope.launch { + accountViewModel.viewModelScope.launch { shareVideoFile( context = context, videoUrl = videoUri, @@ -858,7 +856,7 @@ fun ShareMediaAction( is MediaLocalVideo -> { content.localFile?.let { localFile -> M3ActionRow(icon = Icons.Outlined.Share, text = stringRes(R.string.share_video)) { - viewModelScope.launch { shareLocalVideoFile(context, localFile, mimeType) } + accountViewModel.viewModelScope.launch { shareLocalVideoFile(context, localFile, mimeType) } onDismiss() } } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/ShareHelperTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/ShareHelperTest.kt index 8bbc8a29a..e86d33789 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/ShareHelperTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/ShareHelperTest.kt @@ -159,6 +159,7 @@ class ShareHelperTest { private fun createTempFileWithBytes(bytes: ByteArray): File { val file = File.createTempFile("sharehelpertest", ".tmp") + file.deleteOnExit() file.writeBytes(bytes) return file }