Try avoiding trailing punctuation inside linkified URLs. (#4214)

Create `LinkfierHelper` and post-process URLSpans added to make sure they honor the actual URLs in text by removing unnecessarily added trailing punctuation.

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemTextView.kt

@@ -20,13 +20,16 @@ import androidx.compose.runtime.remember
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.semantics.contentDescription
 import androidx.compose.ui.semantics.semantics
+import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.tooling.preview.PreviewParameter
 import io.element.android.compound.theme.ElementTheme
 import io.element.android.features.messages.impl.timeline.components.layout.ContentAvoidingLayout
 import io.element.android.features.messages.impl.timeline.components.layout.ContentAvoidingLayoutData
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemTextBasedContent
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemTextBasedContentProvider
+import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemTextContent
 import io.element.android.features.messages.impl.utils.containsOnlyEmojis
+import io.element.android.libraries.androidutils.text.LinkifyHelper
 import io.element.android.libraries.designsystem.preview.ElementPreview
 import io.element.android.libraries.designsystem.preview.PreviewsDayNight
 import io.element.android.libraries.matrix.api.core.UserId
@@ -114,3 +117,27 @@ internal fun TimelineItemTextViewPreview(
         onLinkClick = {},
     )
 }
+
+@Preview
+@Composable
+internal fun TimelineItemTextViewWithLinkifiedUrlPreview() = ElementPreview {
+    val content = aTimelineItemTextContent(
+        pillifiedBody = LinkifyHelper.linkify("The link should end after the first '?' (url: github.com/element-hq/element-x-android/README?)?.")
+    )
+    TimelineItemTextView(
+        content = content,
+        onLinkClick = {},
+    )
+}
+
+@Preview
+@Composable
+internal fun TimelineItemTextViewWithLinkifiedUrlAndNestedParenthesisPreview() = ElementPreview {
+    val content = aTimelineItemTextContent(
+        pillifiedBody = LinkifyHelper.linkify("The link should end after the '(ME)' ((url: github.com/element-hq/element-x-android/READ(ME)))!")
+    )
+    TimelineItemTextView(
+        content = content,
+        onLinkClick = {},
+    )
+}

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt

@@ -7,13 +7,10 @@
 
 package io.element.android.features.messages.impl.timeline.factories.event
 
-import android.text.Spannable
 import android.text.style.URLSpan
-import android.text.util.Linkify
 import androidx.core.text.buildSpannedString
 import androidx.core.text.getSpans
 import androidx.core.text.toSpannable
-import androidx.core.text.util.LinkifyCompat
 import io.element.android.features.location.api.Location
 import io.element.android.features.messages.api.timeline.HtmlConverterProvider
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemAudioContent
@@ -29,6 +26,7 @@ import io.element.android.features.messages.impl.timeline.model.event.TimelineIt
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemVoiceContent
 import io.element.android.features.messages.impl.utils.TextPillificationHelper
 import io.element.android.libraries.androidutils.filesize.FileSizeFormatter
+import io.element.android.libraries.androidutils.text.safeLinkify
 import io.element.android.libraries.core.mimetype.MimeTypes
 import io.element.android.libraries.featureflag.api.FeatureFlagService
 import io.element.android.libraries.featureflag.api.FeatureFlags
@@ -232,7 +230,7 @@ class TimelineItemContentMessageFactory @Inject constructor(
                 val body = messageType.body.trimEnd()
                 TimelineItemTextContent(
                     body = body,
-                    pillifiedBody = textPillificationHelper.pillify(body),
+                    pillifiedBody = textPillificationHelper.pillify(body).safeLinkify(),
                     htmlDocument = messageType.formatted?.toHtmlDocument(permalinkParser = permalinkParser),
                     formattedBody = parseHtml(messageType.formatted) ?: body.withLinks(),
                     isEdited = content.isEdited,
@@ -265,7 +263,7 @@ class TimelineItemContentMessageFactory @Inject constructor(
         if (formattedBody == null || formattedBody.format != MessageFormat.HTML) return null
         val result = htmlConverterProvider.provide()
             .fromHtmlToSpans(formattedBody.body.trimEnd())
-            .withFixedURLSpans()
+            .safeLinkify()
         return if (prefix != null) {
             buildSpannedString {
                 append(prefix)
@@ -276,36 +274,11 @@ class TimelineItemContentMessageFactory @Inject constructor(
             result
         }
     }
-
-    private fun CharSequence.withFixedURLSpans(): CharSequence {
-        val spannable = this.toSpannable()
-        // Get all URL spans, as they will be removed by LinkifyCompat.addLinks
-        val oldURLSpans = spannable.getSpans<URLSpan>(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(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 = spannable.getSpans<URLSpan>(start, end).orEmpty()
-            if (addedSpans.isNotEmpty()) {
-                for (span in addedSpans) {
-                    spannable.removeSpan(span)
-                }
-            }
-            spannable.setSpan(urlSpan, start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
-        }
-        return spannable
-    }
 }
 
 @Suppress("USELESS_ELVIS")
 private fun String.withLinks(): CharSequence? {
     // Note: toSpannable() can return null when running unit tests
-    val spannable = toSpannable() ?: return null
-    val addedLinks = LinkifyCompat.addLinks(spannable, Linkify.WEB_URLS or Linkify.PHONE_NUMBERS or Linkify.EMAIL_ADDRESSES)
-    return spannable.takeIf { addedLinks }
+    val spannable = safeLinkify().toSpannable() ?: return null
+    return spannable.takeIf { spannable.getSpans<URLSpan>(0, length).isNotEmpty() }
 }

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactoryTest.kt

@@ -144,6 +144,7 @@ class TimelineItemContentMessageFactoryTest {
             plainText = "body",
             isEdited = false,
             formattedBody = null,
+            pillifiedBody = SpannableString("body"),
         )
         assertThat(result).isEqualTo(expected)
     }

libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/text/LinkifyHelper.kt

@@ -0,0 +1,110 @@
+/*
+ * Copyright 2025 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+package io.element.android.libraries.androidutils.text
+
+import android.text.Spannable
+import android.text.style.URLSpan
+import android.text.util.Linkify
+import androidx.core.text.getSpans
+import androidx.core.text.toSpannable
+import androidx.core.text.util.LinkifyCompat
+import timber.log.Timber
+import kotlin.collections.component1
+import kotlin.collections.component2
+import kotlin.collections.isNotEmpty
+import kotlin.collections.iterator
+
+/**
+ * Helper class to linkify text while preserving existing URL spans.
+ *
+ * It also checks the linkified results to make sure URLs spans are not including trailing punctuation.
+ */
+object LinkifyHelper {
+    fun linkify(
+        text: CharSequence,
+        @LinkifyCompat.LinkifyMask linkifyMask: Int = Linkify.WEB_URLS or Linkify.PHONE_NUMBERS or Linkify.EMAIL_ADDRESSES,
+    ): CharSequence {
+        // Convert the text to a Spannable to be able to add URL spans, return the original text if it's not possible (in tests, i.e.)
+        val spannable = text.toSpannable() ?: return text
+
+        // Get all URL spans, as they will be removed by LinkifyCompat.addLinks
+        val oldURLSpans = spannable.getSpans<URLSpan>(0, text.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
+        val addedNewLinks = LinkifyCompat.addLinks(spannable, linkifyMask)
+
+        // Process newly added URL spans
+        if (addedNewLinks) {
+            val newUrlSpans = spannable.getSpans<URLSpan>(0, spannable.length)
+            for (urlSpan in newUrlSpans) {
+                val start = spannable.getSpanStart(urlSpan)
+                val end = spannable.getSpanEnd(urlSpan)
+
+                // Try to avoid including trailing punctuation in the link.
+                // Since this might fail in some edge cases, we catch the exception and just use the original end index.
+                val newEnd = runCatching {
+                    adjustLinkifiedUrlSpanEndIndex(spannable, start, end)
+                }.onFailure {
+                    Timber.e(it, "Failed to adjust end index for link span")
+                }.getOrNull() ?: end
+
+                // Adapt the url in the URL span to the new end index too if needed
+                if (end != newEnd) {
+                    val url = spannable.subSequence(start, newEnd).toString()
+                    spannable.removeSpan(urlSpan)
+                    spannable.setSpan(URLSpan(url), start, newEnd, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
+                } else {
+                    spannable.setSpan(urlSpan, start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
+                }
+            }
+        }
+
+        // Restore old spans, remove new ones if there is a conflict
+        for ((urlSpan, location) in oldURLSpans) {
+            val (start, end) = location
+            val addedConflictingSpans = spannable.getSpans<URLSpan>(start, end)
+            if (addedConflictingSpans.isNotEmpty()) {
+                for (span in addedConflictingSpans) {
+                    spannable.removeSpan(span)
+                }
+            }
+
+            spannable.setSpan(urlSpan, start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
+        }
+        return spannable
+    }
+
+    private fun adjustLinkifiedUrlSpanEndIndex(spannable: Spannable, start: Int, end: Int): Int {
+        var end = end
+
+        // Trailing punctuation found, adjust the end index
+        while (spannable[end - 1] in sequenceOf('.', ',', ';', ':', '!', '?', '…') && end > start) {
+            end--
+        }
+
+        // If the last character is a closing parenthesis, check if it's part of a pair
+        if (spannable[end - 1] == ')' && end > start) {
+            val linkifiedTextLastPath = spannable.substring(start, end).substringAfterLast('/')
+            val closingParenthesisCount = linkifiedTextLastPath.count { it == ')' }
+            val openingParenthesisCount = linkifiedTextLastPath.count { it == '(' }
+            // If it's not part of a pair, remove it from the link span by adjusting the end index
+            end -= closingParenthesisCount - openingParenthesisCount
+        }
+        return end
+    }
+}
+
+/**
+ * Linkify the text with the default mask (WEB_URLS, PHONE_NUMBERS, EMAIL_ADDRESSES).
+ */
+fun CharSequence.safeLinkify(): CharSequence {
+    return LinkifyHelper.linkify(this, Linkify.WEB_URLS or Linkify.PHONE_NUMBERS or Linkify.EMAIL_ADDRESSES)
+}

libraries/androidutils/src/test/kotlin/io/element/android/libraries/androidutils/text/LinkifierHelperTest.kt

@@ -0,0 +1,124 @@
+/*
+ * Copyright 2025 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+package io.element.android.libraries.androidutils.text
+
+import android.telephony.TelephonyManager
+import android.text.style.URLSpan
+import androidx.core.text.getSpans
+import androidx.core.text.toSpannable
+import com.google.common.truth.Truth.assertThat
+import io.element.android.tests.testutils.WarmUpRule
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.Shadows.shadowOf
+import org.robolectric.annotation.Config
+import org.robolectric.shadow.api.Shadow.newInstanceOf
+
+@RunWith(RobolectricTestRunner::class)
+class LinkifierHelperTest {
+    @get:Rule
+    val warmUpRule = WarmUpRule()
+
+    @Test
+    fun `linkification finds URL`() {
+        val text = "A url https://matrix.org"
+        val result = LinkifyHelper.linkify(text)
+        val urlSpans = result.toSpannable().getSpans<URLSpan>()
+        assertThat(urlSpans.size).isEqualTo(1)
+        assertThat(urlSpans.first().url).isEqualTo("https://matrix.org")
+    }
+
+    @Test
+    fun `linkification finds partial URL`() {
+        val text = "A partial url matrix.org/test"
+        val result = LinkifyHelper.linkify(text)
+        val urlSpans = result.toSpannable().getSpans<URLSpan>()
+        assertThat(urlSpans.size).isEqualTo(1)
+        assertThat(urlSpans.first().url).isEqualTo("http://matrix.org/test")
+    }
+
+    @Test
+    fun `linkification finds domain`() {
+        val text = "A domain matrix.org"
+        val result = LinkifyHelper.linkify(text)
+        val urlSpans = result.toSpannable().getSpans<URLSpan>()
+        assertThat(urlSpans.size).isEqualTo(1)
+        assertThat(urlSpans.first().url).isEqualTo("http://matrix.org")
+    }
+
+    @Test
+    fun `linkification finds email`() {
+        val text = "An email address john@doe.com"
+        val result = LinkifyHelper.linkify(text)
+        val urlSpans = result.toSpannable().getSpans<URLSpan>()
+        assertThat(urlSpans.size).isEqualTo(1)
+        assertThat(urlSpans.first().url).isEqualTo("mailto:john@doe.com")
+    }
+
+    @Test
+    @Config(sdk = [30])
+    fun `linkification finds phone`() {
+        val text = "Test phone number +34950123456"
+        val result = LinkifyHelper.linkify(text)
+        val urlSpans = result.toSpannable().getSpans<URLSpan>()
+        assertThat(urlSpans.size).isEqualTo(1)
+        assertThat(urlSpans.first().url).isEqualTo("tel:+34950123456")
+    }
+
+    @Test
+    @Config(sdk = [30])
+    fun `linkification finds phone in Germany`() {
+        // For some reason the linkification of phone numbers in Germany is very lenient and any number will fit here
+        val telephonyManager = shadowOf(newInstanceOf(TelephonyManager::class.java))
+        telephonyManager.setSimCountryIso("DE")
+
+        val text = "Test phone number 1234"
+        val result = LinkifyHelper.linkify(text)
+        val urlSpans = result.toSpannable().getSpans<URLSpan>()
+        assertThat(urlSpans.size).isEqualTo(1)
+        assertThat(urlSpans.first().url).isEqualTo("tel:1234")
+    }
+
+    @Test
+    fun `linkification handles trailing dot`() {
+        val text = "A url https://matrix.org."
+        val result = LinkifyHelper.linkify(text)
+        val urlSpans = result.toSpannable().getSpans<URLSpan>()
+        assertThat(urlSpans.size).isEqualTo(1)
+        assertThat(urlSpans.first().url).isEqualTo("https://matrix.org")
+    }
+
+    @Test
+    fun `linkification handles trailing punctuation`() {
+        val text = "A url https://matrix.org!?; Check it out!"
+        val result = LinkifyHelper.linkify(text)
+        val urlSpans = result.toSpannable().getSpans<URLSpan>()
+        assertThat(urlSpans.size).isEqualTo(1)
+        assertThat(urlSpans.first().url).isEqualTo("https://matrix.org")
+    }
+
+    @Test
+    fun `linkification handles parenthesis surrounding URL`() {
+        val text = "A url (this one (https://github.com/element-hq/element-android/issues/1234))"
+        val result = LinkifyHelper.linkify(text)
+        val urlSpans = result.toSpannable().getSpans<URLSpan>()
+        assertThat(urlSpans.size).isEqualTo(1)
+        assertThat(urlSpans.first().url).isEqualTo("https://github.com/element-hq/element-android/issues/1234")
+    }
+
+    @Test
+    fun `linkification handles parenthesis in URL`() {
+        val text = "A url: (https://github.com/element-hq/element-android/READ(ME))"
+        val result = LinkifyHelper.linkify(text)
+        val urlSpans = result.toSpannable().getSpans<URLSpan>()
+        assertThat(urlSpans.size).isEqualTo(1)
+        assertThat(urlSpans.first().url).isEqualTo("https://github.com/element-hq/element-android/READ(ME)")
+    }
+}

libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/ClickableLinkText.kt

@@ -147,6 +147,7 @@ fun AnnotatedString.linkify(linkStyle: SpanStyle): AnnotatedString {
             if (original.getLinkAnnotations(start, end).isEmpty() && original.getStringAnnotations("URL", start, end).isEmpty()) {
                 // Prevent linkifying domains in user or room handles (@user:domain.com, #room:domain.com)
                 if (start > 0 && !spannable[start - 1].isWhitespace()) continue
+
                 addStyle(
                     start = start,
                     end = end,