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 48141ccc03..9d9542d595 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 @@ -264,27 +264,27 @@ class TimelineItemContentMessageFactory @Inject constructor( } private fun CharSequence.withFixedURLSpans(): CharSequence { - if (this !is Spannable) return this + val spannable = this.toSpannable() // Get all URL spans, as they will be removed by LinkifyCompat.addLinks - val oldURLSpans = getSpans(0, length).associateWith { - val start = getSpanStart(it) - val end = getSpanEnd(it) + val oldURLSpans = spannable.getSpans(0, length).associateWith { + val start = spannable.getSpanStart(it) + val end = spannable.getSpanEnd(it) Pair(start, end) } // Find and set as URLSpans any links present in the text - LinkifyCompat.addLinks(this, Linkify.WEB_URLS or Linkify.PHONE_NUMBERS or Linkify.EMAIL_ADDRESSES) + LinkifyCompat.addLinks(spannable, Linkify.WEB_URLS or Linkify.PHONE_NUMBERS or Linkify.EMAIL_ADDRESSES) // Restore old spans, remove new ones if there is a conflict for ((urlSpan, location) in oldURLSpans) { val (start, end) = location - val addedSpans = getSpans(start, end).orEmpty() + val addedSpans = spannable.getSpans(start, end).orEmpty() if (addedSpans.isNotEmpty()) { for (span in addedSpans) { - removeSpan(span) + spannable.removeSpan(span) } } - setSpan(urlSpan, start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE) + spannable.setSpan(urlSpan, start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE) } - return this + return spannable } } 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 59cd3cf383..ffb247ed1a 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 @@ -20,9 +20,11 @@ import android.net.Uri import android.text.SpannableString import android.text.SpannableStringBuilder import android.text.Spanned +import android.text.SpannedString import android.text.style.URLSpan import androidx.core.text.buildSpannedString import androidx.core.text.inSpans +import androidx.core.text.toSpannable import com.google.common.truth.Truth.assertThat import io.element.android.features.location.api.Location import io.element.android.features.messages.impl.timeline.model.event.TimelineItemAudioContent @@ -74,6 +76,7 @@ import io.element.android.libraries.mediaviewer.api.util.FileExtensionExtractorW import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.test.runTest +import org.junit.Assert.fail import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -195,7 +198,7 @@ class TimelineItemContentMessageFactoryTest { inSpans(URLSpan("https://matrix.org")) { append("and manually added link") } - } + }.toSpannable() val sut = createTimelineItemContentMessageFactory( htmlConverterTransform = { expected } ) @@ -610,7 +613,7 @@ class TimelineItemContentMessageFactoryTest { senderDisambiguatedDisplayName = "Bob", eventId = AN_EVENT_ID, ) - assertThat((result as TimelineItemNoticeContent).formattedBody).isEqualTo("formatted") + (result as TimelineItemNoticeContent).formattedBody.assertSpannedEquals(SpannedString("formatted")) } @Test @@ -644,7 +647,8 @@ class TimelineItemContentMessageFactoryTest { senderDisambiguatedDisplayName = "Bob", eventId = AN_EVENT_ID, ) - assertThat((result as TimelineItemEmoteContent).formattedBody).isEqualTo(SpannableString("* Bob formatted")) + + (result as TimelineItemEmoteContent).formattedBody.assertSpannedEquals(SpannableString("* Bob formatted")) } @Test @@ -654,7 +658,7 @@ class TimelineItemContentMessageFactoryTest { inSpans(URLSpan("https://www.example.org")) { append("me@matrix.org") } - } + }.toSpannable() val sut = createTimelineItemContentMessageFactory( htmlConverterTransform = { expectedSpanned }, permalinkParser = FakePermalinkParser { PermalinkData.FallbackLink(Uri.EMPTY) } @@ -669,7 +673,59 @@ class TimelineItemContentMessageFactoryTest { senderDisambiguatedDisplayName = "Bob", eventId = AN_EVENT_ID, ) - assertThat((result as TimelineItemTextContent).formattedBody).isEqualTo(expectedSpanned) + (result as TimelineItemTextContent).formattedBody.assertSpannedEquals(expectedSpanned) + } + + @Test + fun `a message with plain URL in a formatted body Spanned format gets linkified too`() = runTest { + val expectedSpanned = buildSpannedString { + append("Test ") + inSpansWithFlags(URLSpan("https://www.example.org"), flags = Spanned.SPAN_EXCLUSIVE_EXCLUSIVE) { + append("https://www.example.org") + } + } + val sut = createTimelineItemContentMessageFactory( + htmlConverterTransform = { expectedSpanned }, + permalinkParser = FakePermalinkParser { PermalinkData.FallbackLink(Uri.EMPTY) } + ) + val result = sut.create( + content = createMessageContent( + type = TextMessageType( + body = "Test [me@matrix.org](https://www.example.org)", + formatted = FormattedBody(MessageFormat.HTML, "Test https://www.example.org") + ) + ), + senderDisambiguatedDisplayName = "Bob", + eventId = AN_EVENT_ID, + ) + (result as TimelineItemTextContent).formattedBody.assertSpannedEquals(expectedSpanned) + } + + @Test + fun `a message with plain URL in a formatted body with plain text format gets linkified too`() = runTest { + val resultString = "Test https://www.example.org" + val expectedSpanned = buildSpannedString { + append("Test ") + inSpansWithFlags(URLSpan("https://www.example.org"), flags = Spanned.SPAN_EXCLUSIVE_EXCLUSIVE) { + append("https://www.example.org") + } + }.toSpannable() + val sut = createTimelineItemContentMessageFactory( + htmlConverterTransform = { resultString }, + permalinkParser = FakePermalinkParser { PermalinkData.FallbackLink(Uri.EMPTY) } + ) + val result = sut.create( + content = createMessageContent( + type = TextMessageType( + body = "Test [me@matrix.org](https://www.example.org)", + formatted = FormattedBody(MessageFormat.HTML, "Test https://www.example.org") + ) + ), + senderDisambiguatedDisplayName = "Bob", + eventId = AN_EVENT_ID, + ) + + (result as TimelineItemTextContent).formattedBody.assertSpannedEquals(expectedSpanned) } private fun createMessageContent( @@ -718,3 +774,40 @@ class TimelineItemContentMessageFactoryTest { fileExtensionExtractor = FileExtensionExtractorWithoutValidation() ) } + +private inline fun SpannableStringBuilder.inSpansWithFlags(span: Any, flags: Int, action: SpannableStringBuilder.() -> Unit) { + val start = this.length + action() + val end = this.length + setSpan(span, start, end, flags) +} + +fun CharSequence?.assertSpannedEquals(other: CharSequence?) { + if (this == null && other == null) { + return + } else if (this is Spanned && other is Spanned) { + assertThat(this.toString()).isEqualTo(other.toString()) + assertThat(this.length).isEqualTo(other.length) + val thisSpans = this.getSpans(0, this.length, Any::class.java) + val otherSpans = other.getSpans(0, other.length, Any::class.java) + if (thisSpans.size != otherSpans.size) { + fail("Expected ${thisSpans.size} spans, got ${otherSpans.size}") + } + thisSpans.forEachIndexed { index, span -> + val otherSpan = otherSpans[index] + // URLSpans don't have a proper `equals` implementation, so we compare the URL instead + if (span is URLSpan && otherSpan is URLSpan) { + assertThat(span.url).isEqualTo(otherSpan.url) + } else { + assertThat(span).isEqualTo(otherSpan) + } + assertThat(this.getSpanStart(span)).isEqualTo(other.getSpanStart(otherSpan)) + assertThat(this.getSpanEnd(span)).isEqualTo(other.getSpanEnd(otherSpan)) + assertThat(this.getSpanFlags(span)).isEqualTo(other.getSpanFlags(otherSpan)) + } + } else { + val thisString = this?.toString() ?: "null" + val otherString = other?.toString() ?: "null" + fail("Expected Spanned, got $thisString and $otherString") + } +}