Merge pull request #5684 from element-hq/feature/bma/notificationCaption

Do not use the bestDescription but the caption for images, when available

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt

@@ -8,7 +8,10 @@
 package io.element.android.libraries.push.impl.notifications
 
 import android.content.Context
+import android.graphics.ImageDecoder
 import android.net.Uri
+import android.os.Build
+import androidx.core.app.NotificationCompat
 import androidx.core.content.FileProvider
 import dev.zacsweers.metro.AppScope
 import dev.zacsweers.metro.ContributesBinding
@@ -138,7 +141,13 @@ class DefaultNotifiableEventResolver(
             is NotificationContent.MessageLike.RoomMessage -> {
                 val showMediaPreview = client.mediaPreviewService.getMediaPreviewValue() == MediaPreviewValue.On
                 val senderDisambiguatedDisplayName = getDisambiguatedDisplayName(content.senderId)
-                val messageBody = descriptionFromMessageContent(content, senderDisambiguatedDisplayName)
+                val imageMimeType = if (showMediaPreview) content.getImageMimetype() else null
+                val imageUriString = imageMimeType?.let { content.fetchImageIfPresent(client, imageMimeType)?.toString() }
+                val messageBody = descriptionFromMessageContent(
+                    content = content,
+                    senderDisambiguatedDisplayName = senderDisambiguatedDisplayName,
+                    hasImageUri = imageUriString != null,
+                )
                 val notifiableMessageEvent = buildNotifiableMessageEvent(
                     sessionId = userId,
                     senderId = content.senderId,
@@ -149,8 +158,8 @@ class DefaultNotifiableEventResolver(
                     timestamp = this.timestamp,
                     senderDisambiguatedDisplayName = senderDisambiguatedDisplayName,
                     body = messageBody,
-                    imageUriString = if (showMediaPreview) content.fetchImageIfPresent(client)?.toString() else null,
+                    imageUriString = imageUriString,
-                    imageMimeType = if (showMediaPreview) content.getImageMimetype() else null,
+                    imageMimeType = imageMimeType.takeIf { imageUriString != null },
                     roomName = roomDisplayName,
                     roomIsDm = isDm,
                     roomAvatarPath = roomAvatarUrl,
@@ -299,13 +308,18 @@ class DefaultNotifiableEventResolver(
     private fun descriptionFromMessageContent(
         content: NotificationContent.MessageLike.RoomMessage,
         senderDisambiguatedDisplayName: String,
-    ): String {
+        hasImageUri: Boolean,
+    ): String? {
         return when (val messageType = content.messageType) {
             is AudioMessageType -> messageType.bestDescription
             is VoiceMessageType -> stringProvider.getString(CommonStrings.common_voice_message)
             is EmoteMessageType -> "* $senderDisambiguatedDisplayName ${messageType.body}"
             is FileMessageType -> messageType.bestDescription
-            is ImageMessageType -> messageType.bestDescription
+            is ImageMessageType -> if (hasImageUri) {
+                messageType.caption
+            } else {
+                messageType.bestDescription
+            }
             is StickerMessageType -> messageType.bestDescription
             is NoticeMessageType -> messageType.body
             is TextMessageType -> messageType.toPlainText(permalinkParser = permalinkParser)
@@ -326,14 +340,34 @@ class DefaultNotifiableEventResolver(
         }
     }
 
-    private suspend fun NotificationContent.MessageLike.RoomMessage.fetchImageIfPresent(client: MatrixClient): Uri? {
+    /**
+     * Fetch the image for message type, only if the mime type is supported, as recommended
+     * per [NotificationCompat.MessagingStyle.Message.setData] documentation.
+     * Then convert to a [Uri] accessible to the Notification Service.
+     */
+    private suspend fun NotificationContent.MessageLike.RoomMessage.fetchImageIfPresent(
+        client: MatrixClient,
+        mimeType: String,
+    ): Uri? {
         val fileResult = when (val messageType = messageType) {
-            is ImageMessageType -> notificationMediaRepoFactory.create(client)
+            is ImageMessageType -> {
-                .getMediaFile(
+                val isMimeTypeSupported = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
-                    mediaSource = messageType.source,
+                    ImageDecoder.isMimeTypeSupported(mimeType)
-                    mimeType = messageType.info?.mimetype,
+                } else {
-                    filename = messageType.filename,
+                    // Assume it's supported on old systems...
-                )
+                    true
+                }
+                if (isMimeTypeSupported) {
+                    notificationMediaRepoFactory.create(client).getMediaFile(
+                        mediaSource = messageType.source,
+                        mimeType = messageType.info?.mimetype,
+                        filename = messageType.filename,
+                    )
+                } else {
+                    Timber.tag(loggerTag.value).d("Mime type $mimeType not supported by the system")
+                    null
+                }
+            }
             is VideoMessageType -> null // Use the thumbnail here?
             else -> null
         }

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt

@@ -431,27 +431,37 @@ class DefaultNotificationCreator(
                     senderPerson
                 )
                 else -> {
-                    val message = MessagingStyle.Message(
+                    if (event.imageMimeType != null && event.imageUri != null) {
-                        event.body?.annotateForDebug(71),
+                        // Image case
-                        event.timestamp,
+                        val message = MessagingStyle.Message(
-                        senderPerson
+                            // This text will not be rendered, but some systems does not render the image
-                    ).also { message ->
+                            // if the text is null
-                        event.imageUri?.let {
+                            stringProvider.getString(CommonStrings.common_image),
-                            message.setData(event.imageMimeType ?: "image/", it)
+                            event.timestamp,
-                        }
+                            senderPerson,
-                        message.extras.putString(MESSAGE_EVENT_ID, event.eventId.value)
-                    }
-                    addMessage(message)
-
-                    // Add additional message for captions
-                    if (event.imageUri != null && event.body != null) {
-                        addMessage(
-                            MessagingStyle.Message(
-                                event.body,
-                                event.timestamp,
-                                senderPerson,
-                            )
                         )
+                            .setData(event.imageMimeType, event.imageUri)
+                        message.extras.putString(MESSAGE_EVENT_ID, event.eventId.value)
+                        addMessage(message)
+                        // Add additional message for captions
+                        if (event.body != null) {
+                            addMessage(
+                                MessagingStyle.Message(
+                                    event.body.annotateForDebug(72),
+                                    event.timestamp,
+                                    senderPerson,
+                                )
+                            )
+                        }
+                    } else {
+                        // Text case
+                        val message = MessagingStyle.Message(
+                            event.body?.annotateForDebug(71),
+                            event.timestamp,
+                            senderPerson
+                        )
+                        message.extras.putString(MESSAGE_EVENT_ID, event.eventId.value)
+                        addMessage(message)
                     }
                 }
             }