Skip to content

Commit 5d8403b

Browse files
jmartinespjmartinesp
authored
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.
1 parent b35feb0 commit 5d8403b

File tree

8 files changed

+274
-32
lines changed

8 files changed

+274
-32
lines changed

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,16 @@ import androidx.compose.runtime.remember
2020
import androidx.compose.ui.Modifier
2121
import androidx.compose.ui.semantics.contentDescription
2222
import androidx.compose.ui.semantics.semantics
23+
import androidx.compose.ui.tooling.preview.Preview
2324
import androidx.compose.ui.tooling.preview.PreviewParameter
2425
import io.element.android.compound.theme.ElementTheme
2526
import io.element.android.features.messages.impl.timeline.components.layout.ContentAvoidingLayout
2627
import io.element.android.features.messages.impl.timeline.components.layout.ContentAvoidingLayoutData
2728
import io.element.android.features.messages.impl.timeline.model.event.TimelineItemTextBasedContent
2829
import io.element.android.features.messages.impl.timeline.model.event.TimelineItemTextBasedContentProvider
30+
import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemTextContent
2931
import io.element.android.features.messages.impl.utils.containsOnlyEmojis
32+
import io.element.android.libraries.androidutils.text.LinkifyHelper
3033
import io.element.android.libraries.designsystem.preview.ElementPreview
3134
import io.element.android.libraries.designsystem.preview.PreviewsDayNight
3235
import io.element.android.libraries.matrix.api.core.UserId
@@ -114,3 +117,27 @@ internal fun TimelineItemTextViewPreview(
114117
onLinkClick = {},
115118
)
116119
}
120+
121+
@Preview
122+
@Composable
123+
internal fun TimelineItemTextViewWithLinkifiedUrlPreview() = ElementPreview {
124+
val content = aTimelineItemTextContent(
125+
pillifiedBody = LinkifyHelper.linkify("The link should end after the first '?' (url: github.com/element-hq/element-x-android/README?)?.")
126+
)
127+
TimelineItemTextView(
128+
content = content,
129+
onLinkClick = {},
130+
)
131+
}
132+
133+
@Preview
134+
@Composable
135+
internal fun TimelineItemTextViewWithLinkifiedUrlAndNestedParenthesisPreview() = ElementPreview {
136+
val content = aTimelineItemTextContent(
137+
pillifiedBody = LinkifyHelper.linkify("The link should end after the '(ME)' ((url: github.com/element-hq/element-x-android/READ(ME)))!")
138+
)
139+
TimelineItemTextView(
140+
content = content,
141+
onLinkClick = {},
142+
)
143+
}

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

Lines changed: 5 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,10 @@
77

88
package io.element.android.features.messages.impl.timeline.factories.event
99

10-
import android.text.Spannable
1110
import android.text.style.URLSpan
12-
import android.text.util.Linkify
1311
import androidx.core.text.buildSpannedString
1412
import androidx.core.text.getSpans
1513
import androidx.core.text.toSpannable
16-
import androidx.core.text.util.LinkifyCompat
1714
import io.element.android.features.location.api.Location
1815
import io.element.android.features.messages.api.timeline.HtmlConverterProvider
1916
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
2926
import io.element.android.features.messages.impl.timeline.model.event.TimelineItemVoiceContent
3027
import io.element.android.features.messages.impl.utils.TextPillificationHelper
3128
import io.element.android.libraries.androidutils.filesize.FileSizeFormatter
29+
import io.element.android.libraries.androidutils.text.safeLinkify
3230
import io.element.android.libraries.core.mimetype.MimeTypes
3331
import io.element.android.libraries.featureflag.api.FeatureFlagService
3432
import io.element.android.libraries.featureflag.api.FeatureFlags
@@ -232,7 +230,7 @@ class TimelineItemContentMessageFactory @Inject constructor(
232230
val body = messageType.body.trimEnd()
233231
TimelineItemTextContent(
234232
body = body,
235-
pillifiedBody = textPillificationHelper.pillify(body),
233+
pillifiedBody = textPillificationHelper.pillify(body).safeLinkify(),
236234
htmlDocument = messageType.formatted?.toHtmlDocument(permalinkParser = permalinkParser),
237235
formattedBody = parseHtml(messageType.formatted) ?: body.withLinks(),
238236
isEdited = content.isEdited,
@@ -265,7 +263,7 @@ class TimelineItemContentMessageFactory @Inject constructor(
265263
if (formattedBody == null || formattedBody.format != MessageFormat.HTML) return null
266264
val result = htmlConverterProvider.provide()
267265
.fromHtmlToSpans(formattedBody.body.trimEnd())
268-
.withFixedURLSpans()
266+
.safeLinkify()
269267
return if (prefix != null) {
270268
buildSpannedString {
271269
append(prefix)
@@ -276,36 +274,11 @@ class TimelineItemContentMessageFactory @Inject constructor(
276274
result
277275
}
278276
}
279-
280-
private fun CharSequence.withFixedURLSpans(): CharSequence {
281-
val spannable = this.toSpannable()
282-
// Get all URL spans, as they will be removed by LinkifyCompat.addLinks
283-
val oldURLSpans = spannable.getSpans<URLSpan>(0, length).associateWith {
284-
val start = spannable.getSpanStart(it)
285-
val end = spannable.getSpanEnd(it)
286-
Pair(start, end)
287-
}
288-
// Find and set as URLSpans any links present in the text
289-
LinkifyCompat.addLinks(spannable, Linkify.WEB_URLS or Linkify.PHONE_NUMBERS or Linkify.EMAIL_ADDRESSES)
290-
// Restore old spans, remove new ones if there is a conflict
291-
for ((urlSpan, location) in oldURLSpans) {
292-
val (start, end) = location
293-
val addedSpans = spannable.getSpans<URLSpan>(start, end).orEmpty()
294-
if (addedSpans.isNotEmpty()) {
295-
for (span in addedSpans) {
296-
spannable.removeSpan(span)
297-
}
298-
}
299-
spannable.setSpan(urlSpan, start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
300-
}
301-
return spannable
302-
}
303277
}
304278

305279
@Suppress("USELESS_ELVIS")
306280
private fun String.withLinks(): CharSequence? {
307281
// Note: toSpannable() can return null when running unit tests
308-
val spannable = toSpannable() ?: return null
309-
val addedLinks = LinkifyCompat.addLinks(spannable, Linkify.WEB_URLS or Linkify.PHONE_NUMBERS or Linkify.EMAIL_ADDRESSES)
310-
return spannable.takeIf { addedLinks }
282+
val spannable = safeLinkify().toSpannable() ?: return null
283+
return spannable.takeIf { spannable.getSpans<URLSpan>(0, length).isNotEmpty() }
311284
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ class TimelineItemContentMessageFactoryTest {
144144
plainText = "body",
145145
isEdited = false,
146146
formattedBody = null,
147+
pillifiedBody = SpannableString("body"),
147148
)
148149
assertThat(result).isEqualTo(expected)
149150
}

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/*
2+
* Copyright 2025 New Vector Ltd.
3+
*
4+
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
5+
* Please see LICENSE files in the repository root for full details.
6+
*/
7+
8+
package io.element.android.libraries.androidutils.text
9+
10+
import android.text.Spannable
11+
import android.text.style.URLSpan
12+
import android.text.util.Linkify
13+
import androidx.core.text.getSpans
14+
import androidx.core.text.toSpannable
15+
import androidx.core.text.util.LinkifyCompat
16+
import timber.log.Timber
17+
import kotlin.collections.component1
18+
import kotlin.collections.component2
19+
import kotlin.collections.isNotEmpty
20+
import kotlin.collections.iterator
21+
22+
/**
23+
* Helper class to linkify text while preserving existing URL spans.
24+
*
25+
* It also checks the linkified results to make sure URLs spans are not including trailing punctuation.
26+
*/
27+
object LinkifyHelper {
28+
fun linkify(
29+
text: CharSequence,
30+
@LinkifyCompat.LinkifyMask linkifyMask: Int = Linkify.WEB_URLS or Linkify.PHONE_NUMBERS or Linkify.EMAIL_ADDRESSES,
31+
): CharSequence {
32+
// 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.)
33+
val spannable = text.toSpannable() ?: return text
34+
35+
// Get all URL spans, as they will be removed by LinkifyCompat.addLinks
36+
val oldURLSpans = spannable.getSpans<URLSpan>(0, text.length).associateWith {
37+
val start = spannable.getSpanStart(it)
38+
val end = spannable.getSpanEnd(it)
39+
Pair(start, end)
40+
}
41+
// Find and set as URLSpans any links present in the text
42+
val addedNewLinks = LinkifyCompat.addLinks(spannable, linkifyMask)
43+
44+
// Process newly added URL spans
45+
if (addedNewLinks) {
46+
val newUrlSpans = spannable.getSpans<URLSpan>(0, spannable.length)
47+
for (urlSpan in newUrlSpans) {
48+
val start = spannable.getSpanStart(urlSpan)
49+
val end = spannable.getSpanEnd(urlSpan)
50+
51+
// Try to avoid including trailing punctuation in the link.
52+
// Since this might fail in some edge cases, we catch the exception and just use the original end index.
53+
val newEnd = runCatching {
54+
adjustLinkifiedUrlSpanEndIndex(spannable, start, end)
55+
}.onFailure {
56+
Timber.e(it, "Failed to adjust end index for link span")
57+
}.getOrNull() ?: end
58+
59+
// Adapt the url in the URL span to the new end index too if needed
60+
if (end != newEnd) {
61+
val url = spannable.subSequence(start, newEnd).toString()
62+
spannable.removeSpan(urlSpan)
63+
spannable.setSpan(URLSpan(url), start, newEnd, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
64+
} else {
65+
spannable.setSpan(urlSpan, start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
66+
}
67+
}
68+
}
69+
70+
// Restore old spans, remove new ones if there is a conflict
71+
for ((urlSpan, location) in oldURLSpans) {
72+
val (start, end) = location
73+
val addedConflictingSpans = spannable.getSpans<URLSpan>(start, end)
74+
if (addedConflictingSpans.isNotEmpty()) {
75+
for (span in addedConflictingSpans) {
76+
spannable.removeSpan(span)
77+
}
78+
}
79+
80+
spannable.setSpan(urlSpan, start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
81+
}
82+
return spannable
83+
}
84+
85+
private fun adjustLinkifiedUrlSpanEndIndex(spannable: Spannable, start: Int, end: Int): Int {
86+
var end = end
87+
88+
// Trailing punctuation found, adjust the end index
89+
while (spannable[end - 1] in sequenceOf('.', ',', ';', ':', '!', '?', '…') && end > start) {
90+
end--
91+
}
92+
93+
// If the last character is a closing parenthesis, check if it's part of a pair
94+
if (spannable[end - 1] == ')' && end > start) {
95+
val linkifiedTextLastPath = spannable.substring(start, end).substringAfterLast('/')
96+
val closingParenthesisCount = linkifiedTextLastPath.count { it == ')' }
97+
val openingParenthesisCount = linkifiedTextLastPath.count { it == '(' }
98+
// If it's not part of a pair, remove it from the link span by adjusting the end index
99+
end -= closingParenthesisCount - openingParenthesisCount
100+
}
101+
return end
102+
}
103+
}
104+
105+
/**
106+
* Linkify the text with the default mask (WEB_URLS, PHONE_NUMBERS, EMAIL_ADDRESSES).
107+
*/
108+
fun CharSequence.safeLinkify(): CharSequence {
109+
return LinkifyHelper.linkify(this, Linkify.WEB_URLS or Linkify.PHONE_NUMBERS or Linkify.EMAIL_ADDRESSES)
110+
}

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

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/*
2+
* Copyright 2025 New Vector Ltd.
3+
*
4+
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
5+
* Please see LICENSE files in the repository root for full details.
6+
*/
7+
8+
package io.element.android.libraries.androidutils.text
9+
10+
import android.telephony.TelephonyManager
11+
import android.text.style.URLSpan
12+
import androidx.core.text.getSpans
13+
import androidx.core.text.toSpannable
14+
import com.google.common.truth.Truth.assertThat
15+
import io.element.android.tests.testutils.WarmUpRule
16+
import org.junit.Rule
17+
import org.junit.Test
18+
import org.junit.runner.RunWith
19+
import org.robolectric.RobolectricTestRunner
20+
import org.robolectric.Shadows.shadowOf
21+
import org.robolectric.annotation.Config
22+
import org.robolectric.shadow.api.Shadow.newInstanceOf
23+
24+
@RunWith(RobolectricTestRunner::class)
25+
class LinkifierHelperTest {
26+
@get:Rule
27+
val warmUpRule = WarmUpRule()
28+
29+
@Test
30+
fun `linkification finds URL`() {
31+
val text = "A url https://matrix.org"
32+
val result = LinkifyHelper.linkify(text)
33+
val urlSpans = result.toSpannable().getSpans<URLSpan>()
34+
assertThat(urlSpans.size).isEqualTo(1)
35+
assertThat(urlSpans.first().url).isEqualTo("https://matrix.org")
36+
}
37+
38+
@Test
39+
fun `linkification finds partial URL`() {
40+
val text = "A partial url matrix.org/test"
41+
val result = LinkifyHelper.linkify(text)
42+
val urlSpans = result.toSpannable().getSpans<URLSpan>()
43+
assertThat(urlSpans.size).isEqualTo(1)
44+
assertThat(urlSpans.first().url).isEqualTo("http://matrix.org/test")
45+
}
46+
47+
@Test
48+
fun `linkification finds domain`() {
49+
val text = "A domain matrix.org"
50+
val result = LinkifyHelper.linkify(text)
51+
val urlSpans = result.toSpannable().getSpans<URLSpan>()
52+
assertThat(urlSpans.size).isEqualTo(1)
53+
assertThat(urlSpans.first().url).isEqualTo("http://matrix.org")
54+
}
55+
56+
@Test
57+
fun `linkification finds email`() {
58+
val text = "An email address [email protected]"
59+
val result = LinkifyHelper.linkify(text)
60+
val urlSpans = result.toSpannable().getSpans<URLSpan>()
61+
assertThat(urlSpans.size).isEqualTo(1)
62+
assertThat(urlSpans.first().url).isEqualTo("mailto:[email protected]")
63+
}
64+
65+
@Test
66+
@Config(sdk = [30])
67+
fun `linkification finds phone`() {
68+
val text = "Test phone number +34950123456"
69+
val result = LinkifyHelper.linkify(text)
70+
val urlSpans = result.toSpannable().getSpans<URLSpan>()
71+
assertThat(urlSpans.size).isEqualTo(1)
72+
assertThat(urlSpans.first().url).isEqualTo("tel:+34950123456")
73+
}
74+
75+
@Test
76+
@Config(sdk = [30])
77+
fun `linkification finds phone in Germany`() {
78+
// For some reason the linkification of phone numbers in Germany is very lenient and any number will fit here
79+
val telephonyManager = shadowOf(newInstanceOf(TelephonyManager::class.java))
80+
telephonyManager.setSimCountryIso("DE")
81+
82+
val text = "Test phone number 1234"
83+
val result = LinkifyHelper.linkify(text)
84+
val urlSpans = result.toSpannable().getSpans<URLSpan>()
85+
assertThat(urlSpans.size).isEqualTo(1)
86+
assertThat(urlSpans.first().url).isEqualTo("tel:1234")
87+
}
88+
89+
@Test
90+
fun `linkification handles trailing dot`() {
91+
val text = "A url https://matrix.org."
92+
val result = LinkifyHelper.linkify(text)
93+
val urlSpans = result.toSpannable().getSpans<URLSpan>()
94+
assertThat(urlSpans.size).isEqualTo(1)
95+
assertThat(urlSpans.first().url).isEqualTo("https://matrix.org")
96+
}
97+
98+
@Test
99+
fun `linkification handles trailing punctuation`() {
100+
val text = "A url https://matrix.org!?; Check it out!"
101+
val result = LinkifyHelper.linkify(text)
102+
val urlSpans = result.toSpannable().getSpans<URLSpan>()
103+
assertThat(urlSpans.size).isEqualTo(1)
104+
assertThat(urlSpans.first().url).isEqualTo("https://matrix.org")
105+
}
106+
107+
@Test
108+
fun `linkification handles parenthesis surrounding URL`() {
109+
val text = "A url (this one (https://github.com/element-hq/element-android/issues/1234))"
110+
val result = LinkifyHelper.linkify(text)
111+
val urlSpans = result.toSpannable().getSpans<URLSpan>()
112+
assertThat(urlSpans.size).isEqualTo(1)
113+
assertThat(urlSpans.first().url).isEqualTo("https://github.com/element-hq/element-android/issues/1234")
114+
}
115+
116+
@Test
117+
fun `linkification handles parenthesis in URL`() {
118+
val text = "A url: (https://github.com/element-hq/element-android/READ(ME))"
119+
val result = LinkifyHelper.linkify(text)
120+
val urlSpans = result.toSpannable().getSpans<URLSpan>()
121+
assertThat(urlSpans.size).isEqualTo(1)
122+
assertThat(urlSpans.first().url).isEqualTo("https://github.com/element-hq/element-android/READ(ME)")
123+
}
124+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ fun AnnotatedString.linkify(linkStyle: SpanStyle): AnnotatedString {
147147
if (original.getLinkAnnotations(start, end).isEmpty() && original.getStringAnnotations("URL", start, end).isEmpty()) {
148148
// Prevent linkifying domains in user or room handles (@user:domain.com, #room:domain.com)
149149
if (start > 0 && !spannable[start - 1].isWhitespace()) continue
150+
150151
addStyle(
151152
start = start,
152153
end = end,

‎tests/uitests/src/test/snapshots/images/features.messages.impl.timeline.components.event_TimelineItemTextViewWithLinkifiedUrlAndNestedParenthesis_en.png‎

Lines changed: 3 additions & 0 deletions
Loading

‎tests/uitests/src/test/snapshots/images/features.messages.impl.timeline.components.event_TimelineItemTextViewWithLinkifiedUrl_en.png‎

Lines changed: 3 additions & 0 deletions
Loading

0 commit comments

Comments
 (0)