From 2ca981ddfa6f3bf426e76be1ec8bf6d6f96a74d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Wed, 17 Dec 2025 18:38:02 +0100 Subject: [PATCH] Reuse already parsed document instead of parsing it again --- .../TimelineItemContentMessageFactory.kt | 81 ++++++++++--------- .../TimelineItemContentMessageFactoryTest.kt | 8 +- .../timeline/FakeHtmlConverterProvider.kt | 6 ++ libraries/matrixui/build.gradle.kts | 2 +- .../matrix/ui/messages/ToHtmlDocument.kt | 6 +- 5 files changed, 59 insertions(+), 44 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt index a3c8a1bb26..3d8467729a 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt @@ -9,7 +9,6 @@ package io.element.android.features.messages.impl.timeline.factories.event import android.text.style.URLSpan -import androidx.core.text.buildSpannedString import androidx.core.text.getSpans import androidx.core.text.toSpannable import dev.zacsweers.metro.Inject @@ -35,11 +34,9 @@ import io.element.android.libraries.matrix.api.permalink.PermalinkParser import io.element.android.libraries.matrix.api.timeline.item.event.AudioMessageType import io.element.android.libraries.matrix.api.timeline.item.event.EmoteMessageType import io.element.android.libraries.matrix.api.timeline.item.event.FileMessageType -import io.element.android.libraries.matrix.api.timeline.item.event.FormattedBody import io.element.android.libraries.matrix.api.timeline.item.event.ImageMessageType import io.element.android.libraries.matrix.api.timeline.item.event.LocationMessageType import io.element.android.libraries.matrix.api.timeline.item.event.MessageContent -import io.element.android.libraries.matrix.api.timeline.item.event.MessageFormat import io.element.android.libraries.matrix.api.timeline.item.event.NoticeMessageType import io.element.android.libraries.matrix.api.timeline.item.event.OtherMessageType import io.element.android.libraries.matrix.api.timeline.item.event.StickerMessageType @@ -50,6 +47,7 @@ import io.element.android.libraries.matrix.ui.messages.toHtmlDocument import io.element.android.libraries.mediaviewer.api.util.FileExtensionExtractor import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList +import org.jsoup.nodes.Document import kotlin.time.Duration @Inject @@ -60,7 +58,7 @@ class TimelineItemContentMessageFactory( private val permalinkParser: PermalinkParser, private val textPillificationHelper: TextPillificationHelper, ) { - suspend fun create( + fun create( content: MessageContent, senderDisambiguatedDisplayName: String, eventId: EventId?, @@ -68,26 +66,29 @@ class TimelineItemContentMessageFactory( return when (val messageType = content.type) { is EmoteMessageType -> { val emoteBody = "* $senderDisambiguatedDisplayName ${messageType.body.trimEnd()}" - val formattedBody = parseHtml(messageType.formatted, prefix = "* $senderDisambiguatedDisplayName") ?: textPillificationHelper.pillify( - emoteBody - ).safeLinkify() + val dom = messageType.formatted?.toHtmlDocument( + permalinkParser = permalinkParser, + prefix = "* $senderDisambiguatedDisplayName", + ) + val formattedBody = dom?.let(::parseHtml) + ?: textPillificationHelper.pillify(emoteBody).safeLinkify() TimelineItemEmoteContent( body = emoteBody, - htmlDocument = messageType.formatted?.toHtmlDocument( - permalinkParser = permalinkParser, - prefix = "* $senderDisambiguatedDisplayName", - ), + htmlDocument = dom, formattedBody = formattedBody, isEdited = content.isEdited, ) } is ImageMessageType -> { + val dom = messageType.formattedCaption?.toHtmlDocument(permalinkParser = permalinkParser) + val formattedCaption = dom?.let(::parseHtml) + ?: messageType.caption?.withLinks() val aspectRatio = aspectRatioOf(messageType.info?.width, messageType.info?.height) TimelineItemImageContent( filename = messageType.filename, fileSize = messageType.info?.size ?: 0, caption = messageType.caption?.trimEnd(), - formattedCaption = parseHtml(messageType.formattedCaption) ?: messageType.caption?.withLinks(), + formattedCaption = formattedCaption, isEdited = content.isEdited, mediaSource = messageType.source, thumbnailSource = messageType.info?.thumbnailSource, @@ -103,12 +104,15 @@ class TimelineItemContentMessageFactory( ) } is StickerMessageType -> { + val dom = messageType.formattedCaption?.toHtmlDocument(permalinkParser = permalinkParser) + val formattedCaption = dom?.let(::parseHtml) + ?: messageType.caption?.withLinks() val aspectRatio = aspectRatioOf(messageType.info?.width, messageType.info?.height) TimelineItemStickerContent( filename = messageType.filename, fileSize = messageType.info?.size ?: 0, caption = messageType.caption?.trimEnd(), - formattedCaption = parseHtml(messageType.formattedCaption) ?: messageType.caption?.withLinks(), + formattedCaption = formattedCaption, isEdited = content.isEdited, mediaSource = messageType.source, thumbnailSource = messageType.info?.thumbnailSource, @@ -140,12 +144,15 @@ class TimelineItemContentMessageFactory( } } is VideoMessageType -> { + val dom = messageType.formattedCaption?.toHtmlDocument(permalinkParser = permalinkParser) + val formattedCaption = dom?.let(::parseHtml) + ?: messageType.caption?.withLinks() val aspectRatio = aspectRatioOf(messageType.info?.width, messageType.info?.height) TimelineItemVideoContent( filename = messageType.filename, fileSize = messageType.info?.size ?: 0, caption = messageType.caption?.trimEnd(), - formattedCaption = parseHtml(messageType.formattedCaption) ?: messageType.caption?.withLinks(), + formattedCaption = formattedCaption, isEdited = content.isEdited, thumbnailSource = messageType.info?.thumbnailSource, mediaSource = messageType.source, @@ -162,11 +169,14 @@ class TimelineItemContentMessageFactory( ) } is AudioMessageType -> { + val dom = messageType.formattedCaption?.toHtmlDocument(permalinkParser = permalinkParser) + val formattedCaption = dom?.let(::parseHtml) + ?: messageType.caption?.withLinks() TimelineItemAudioContent( filename = messageType.filename, fileSize = messageType.info?.size ?: 0, caption = messageType.caption?.trimEnd(), - formattedCaption = parseHtml(messageType.formattedCaption) ?: messageType.caption?.withLinks(), + formattedCaption = formattedCaption, isEdited = content.isEdited, mediaSource = messageType.source, duration = messageType.info?.duration ?: Duration.ZERO, @@ -176,12 +186,15 @@ class TimelineItemContentMessageFactory( ) } is VoiceMessageType -> { + val dom = messageType.formattedCaption?.toHtmlDocument(permalinkParser = permalinkParser) + val formattedCaption = dom?.let(::parseHtml) + ?: messageType.caption?.withLinks() TimelineItemVoiceContent( eventId = eventId, filename = messageType.filename, fileSize = messageType.info?.size ?: 0, caption = messageType.caption?.trimEnd(), - formattedCaption = parseHtml(messageType.formattedCaption) ?: messageType.caption?.withLinks(), + formattedCaption = formattedCaption, isEdited = content.isEdited, mediaSource = messageType.source, duration = messageType.info?.duration ?: Duration.ZERO, @@ -192,12 +205,15 @@ class TimelineItemContentMessageFactory( ) } is FileMessageType -> { + val dom = messageType.formattedCaption?.toHtmlDocument(permalinkParser = permalinkParser) + val formattedCaption = dom?.let(::parseHtml) + ?: messageType.caption?.withLinks() val fileExtension = fileExtensionExtractor.extractFromName(messageType.filename) TimelineItemFileContent( filename = messageType.filename, fileSize = messageType.info?.size ?: 0, caption = messageType.caption?.trimEnd(), - formattedCaption = parseHtml(messageType.formattedCaption) ?: messageType.caption?.withLinks(), + formattedCaption = formattedCaption, isEdited = content.isEdited, thumbnailSource = messageType.info?.thumbnailSource, mediaSource = messageType.source, @@ -208,9 +224,9 @@ class TimelineItemContentMessageFactory( } is NoticeMessageType -> { val body = messageType.body.trimEnd() - val formattedBody = parseHtml(messageType.formatted) ?: textPillificationHelper.pillify( - body - ).safeLinkify() + val dom = messageType.formatted?.toHtmlDocument(permalinkParser = permalinkParser) + val formattedBody = dom?.let(::parseHtml) + ?: textPillificationHelper.pillify(body).safeLinkify() val htmlDocument = messageType.formatted?.toHtmlDocument(permalinkParser = permalinkParser) TimelineItemNoticeContent( body = body, @@ -221,12 +237,13 @@ class TimelineItemContentMessageFactory( } is TextMessageType -> { val body = messageType.body.trimEnd() - val formattedBody = parseHtml(messageType.formatted) ?: textPillificationHelper.pillify( - body - ).safeLinkify() + val dom = messageType.formatted?.toHtmlDocument(permalinkParser = permalinkParser) + val formattedBody = dom?.let(::parseHtml) + ?: textPillificationHelper.pillify(body).safeLinkify() + val htmlDocument = messageType.formatted?.toHtmlDocument(permalinkParser = permalinkParser) TimelineItemTextContent( body = body, - htmlDocument = messageType.formatted?.toHtmlDocument(permalinkParser = permalinkParser), + htmlDocument = htmlDocument, formattedBody = formattedBody, isEdited = content.isEdited, ) @@ -253,21 +270,11 @@ class TimelineItemContentMessageFactory( return result?.takeIf { it.isFinite() } } - private fun parseHtml(formattedBody: FormattedBody?, prefix: String? = null): CharSequence? { - if (formattedBody == null || formattedBody.format != MessageFormat.HTML) return null - val result = htmlConverterProvider.provide() - .fromHtmlToSpans(formattedBody.body.trimEnd()) + private fun parseHtml(document: Document): CharSequence? { + return htmlConverterProvider.provide() + .fromDocumentToSpans(document) .let { textPillificationHelper.pillify(it) } .safeLinkify() - return if (prefix != null) { - buildSpannedString { - append(prefix) - append(" ") - append(result) - } - } else { - result - } } } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactoryTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactoryTest.kt index 160e368916..9f23388ecb 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactoryTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactoryTest.kt @@ -67,6 +67,7 @@ import io.element.android.libraries.mediaviewer.test.util.FileExtensionExtractor import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.test.runTest +import org.jsoup.nodes.Document import org.junit.Assert.fail import org.junit.Test import org.junit.runner.RunWith @@ -187,7 +188,7 @@ class TimelineItemContentMessageFactoryTest { } }.toSpannable() val sut = createTimelineItemContentMessageFactory( - htmlConverterTransform = { expected } + domConverterTransform = { expected } ) val result = sut.create( content = createMessageContent( @@ -679,7 +680,7 @@ class TimelineItemContentMessageFactoryTest { } }.toSpannable() val sut = createTimelineItemContentMessageFactory( - htmlConverterTransform = { expectedSpanned }, + domConverterTransform = { expectedSpanned }, permalinkParser = FakePermalinkParser { PermalinkData.FallbackLink(Uri.EMPTY) } ) val result = sut.create( @@ -765,11 +766,12 @@ class TimelineItemContentMessageFactoryTest { private fun createTimelineItemContentMessageFactory( htmlConverterTransform: (String) -> CharSequence = { it }, + domConverterTransform: (Document) -> CharSequence = { it.body().html() }, permalinkParser: FakePermalinkParser = FakePermalinkParser(), ) = TimelineItemContentMessageFactory( fileSizeFormatter = FakeFileSizeFormatter(), fileExtensionExtractor = FileExtensionExtractorWithoutValidation(), - htmlConverterProvider = FakeHtmlConverterProvider(htmlConverterTransform), + htmlConverterProvider = FakeHtmlConverterProvider(htmlConverterTransform, domConverterTransform), permalinkParser = permalinkParser, textPillificationHelper = FakeTextPillificationHelper(), ) diff --git a/features/messages/test/src/main/kotlin/io/element/android/features/messages/test/timeline/FakeHtmlConverterProvider.kt b/features/messages/test/src/main/kotlin/io/element/android/features/messages/test/timeline/FakeHtmlConverterProvider.kt index 75b700b58d..1277783f6a 100644 --- a/features/messages/test/src/main/kotlin/io/element/android/features/messages/test/timeline/FakeHtmlConverterProvider.kt +++ b/features/messages/test/src/main/kotlin/io/element/android/features/messages/test/timeline/FakeHtmlConverterProvider.kt @@ -11,9 +11,11 @@ package io.element.android.features.messages.test.timeline import androidx.compose.runtime.Composable import io.element.android.features.messages.api.timeline.HtmlConverterProvider import io.element.android.wysiwyg.utils.HtmlConverter +import org.jsoup.nodes.Document class FakeHtmlConverterProvider( private val transform: (String) -> CharSequence = { it }, + private val transformDom: (Document) -> CharSequence = { it.html() }, ) : HtmlConverterProvider { @Composable override fun Update() = Unit @@ -23,6 +25,10 @@ class FakeHtmlConverterProvider( override fun fromHtmlToSpans(html: String): CharSequence { return transform(html) } + + override fun fromDocumentToSpans(dom: Document): CharSequence { + return transformDom(dom) + } } } } diff --git a/libraries/matrixui/build.gradle.kts b/libraries/matrixui/build.gradle.kts index 94aa388c0c..5513dc240e 100644 --- a/libraries/matrixui/build.gradle.kts +++ b/libraries/matrixui/build.gradle.kts @@ -36,7 +36,7 @@ dependencies { implementation(projects.libraries.uiStrings) implementation(projects.libraries.testtags) implementation(libs.coil.compose) - implementation(libs.jsoup) + implementation(libs.matrix.richtexteditor) implementation(projects.libraries.previewutils) testCommonDependencies(libs, true) diff --git a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/messages/ToHtmlDocument.kt b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/messages/ToHtmlDocument.kt index 5a2297ec1f..ee9de51681 100644 --- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/messages/ToHtmlDocument.kt +++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/messages/ToHtmlDocument.kt @@ -12,7 +12,7 @@ import io.element.android.libraries.matrix.api.permalink.PermalinkData import io.element.android.libraries.matrix.api.permalink.PermalinkParser import io.element.android.libraries.matrix.api.timeline.item.event.FormattedBody import io.element.android.libraries.matrix.api.timeline.item.event.MessageFormat -import org.jsoup.Jsoup +import io.element.android.wysiwyg.utils.HtmlToDomParser import org.jsoup.nodes.Document /** @@ -34,9 +34,9 @@ fun FormattedBody.toHtmlDocument( ?.trimEnd() ?.let { formattedBody -> val dom = if (prefix != null) { - Jsoup.parse("$prefix $formattedBody") + HtmlToDomParser.document("$prefix $formattedBody") } else { - Jsoup.parse(formattedBody) + HtmlToDomParser.document(formattedBody) } // Prepend `@` to mentions