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.
This commit is contained in:
Jorge Martin Espinosa 2025-02-21 17:58:59 +01:00 committed by GitHub
parent b35feb0409
commit 5d8403b310
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 274 additions and 32 deletions

View file

@ -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 = {},
)
}

View file

@ -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() }
}

View file

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