From f9f056bf85989e15d5164231183010be3a5c8168 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 5 Nov 2025 11:00:13 +0100 Subject: [PATCH] Do not use the bastDescription but the cation for image/video/sticker because else the filename will be rendered in the notification and for media we do not want that. Also fixes the issue when images is not rendered on some system and so they can be empty notification. Closes #3945 --- .../DefaultNotifiableEventResolver.kt | 58 +++++++++++++++---- .../factories/NotificationCreator.kt | 50 +++++++++------- 2 files changed, 76 insertions(+), 32 deletions(-) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt index 481d354e8d..5e397c94d9 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt +++ b/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, - imageMimeType = if (showMediaPreview) content.getImageMimetype() else null, + imageUriString = imageUriString, + 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) - .getMediaFile( - mediaSource = messageType.source, - mimeType = messageType.info?.mimetype, - filename = messageType.filename, - ) + is ImageMessageType -> { + val isMimeTypeSupported = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + ImageDecoder.isMimeTypeSupported(mimeType) + } else { + // 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 } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt index efe54bd3d9..0dccdb46c2 100755 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt +++ b/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( - event.body?.annotateForDebug(71), - event.timestamp, - senderPerson - ).also { message -> - event.imageUri?.let { - message.setData(event.imageMimeType ?: "image/", it) - } - 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, - ) + if (event.imageMimeType != null && event.imageUri != null) { + // Image case + val message = MessagingStyle.Message( + // This text will not be rendered, but some systems does not render the image + // if the text is null + stringProvider.getString(CommonStrings.common_image), + 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) } } }