Fix linkification not working for Spanned strings in text messages (#3233)

* Fix linkification not working for `Spanned` string instead of `Spannable`.

This issue was found as a regression after upgrading the RTE version to `2.37.7`.

* Fix and add tests

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<URLSpan>(0, length).associateWith {
+        val oldURLSpans = spannable.getSpans<URLSpan>(0, length).associateWith {
-            val start = getSpanStart(it)
+            val start = spannable.getSpanStart(it)
-            val end = getSpanEnd(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<URLSpan>(start, end).orEmpty()
+            val addedSpans = spannable.getSpans<URLSpan>(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
     }
 }
 

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")
+    }
+}