fix how emojis in link texts are displayed (#4723)

![Screenshot_20241010_204357](https://github.com/user-attachments/assets/6f511231-b89c-4902-ad89-68c1940415a7)

closes https://github.com/tuskyapp/Tusky/issues/4720
This commit is contained in:
Konrad Pozniak 2024-10-11 11:25:43 +02:00 committed by GitHub
parent 0bc227b0f3
commit f8cf38c81b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 41 additions and 37 deletions

View File

@ -115,29 +115,28 @@ fun markupHiddenUrls(view: TextView, content: CharSequence): SpannableStringBuil
for (span in obscuredLinkSpans) { for (span in obscuredLinkSpans) {
val start = spannableContent.getSpanStart(span) val start = spannableContent.getSpanStart(span)
val end = spannableContent.getSpanEnd(span) val end = spannableContent.getSpanEnd(span)
val originalText = spannableContent.subSequence(start, end) val additionalText = " " + view.context.getString(
val replacementText = view.context.getString(
R.string.url_domain_notifier, R.string.url_domain_notifier,
originalText,
getDomain(span.url) getDomain(span.url)
) )
spannableContent.replace( spannableContent.insert(
start,
end, end,
replacementText additionalText
) // this also updates the span locations )
// reinsert the span so it covers the original and the additional text
spannableContent.setSpan(span, start, end + additionalText.length, 0)
val linkDrawable = AppCompatResources.getDrawable(view.context, R.drawable.ic_link)!! val linkDrawable = AppCompatResources.getDrawable(view.context, R.drawable.ic_link)!!
// ImageSpan does not always align the icon correctly in the line, let's use our custom emoji span for this // ImageSpan does not always align the icon correctly in the line, let's use our custom emoji span for this
val linkDrawableSpan = EmojiSpan(view) val linkDrawableSpan = EmojiSpan(view)
linkDrawableSpan.imageDrawable = linkDrawable linkDrawableSpan.imageDrawable = linkDrawable
val placeholderIndex = originalText.length + 2 val placeholderIndex = end + 2
spannableContent.setSpan( spannableContent.setSpan(
linkDrawableSpan, linkDrawableSpan,
start + placeholderIndex, placeholderIndex,
start + placeholderIndex + "🔗".length, placeholderIndex + "🔗".length,
0 0
) )
} }

View File

@ -265,6 +265,6 @@
<item>NEWEST_FIRST</item> <item>NEWEST_FIRST</item>
</string-array> </string-array>
<string name="url_domain_notifier" translatable="false">%1$s (🔗 %2$s)</string> <string name="url_domain_notifier" translatable="false">(🔗 %1$s)</string>
</resources> </resources>

View File

@ -9,7 +9,11 @@ import com.keylesspalace.tusky.R
import com.keylesspalace.tusky.entity.HashTag import com.keylesspalace.tusky.entity.HashTag
import com.keylesspalace.tusky.entity.Status import com.keylesspalace.tusky.entity.Status
import com.keylesspalace.tusky.interfaces.LinkListener import com.keylesspalace.tusky.interfaces.LinkListener
import org.junit.Assert import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test import org.junit.Test
import org.junit.runner.RunWith import org.junit.runner.RunWith
import org.junit.runners.Parameterized import org.junit.runners.Parameterized
@ -51,7 +55,7 @@ class LinkHelperTest {
urlSpans = builder.getSpans(0, builder.length, URLSpan::class.java) urlSpans = builder.getSpans(0, builder.length, URLSpan::class.java)
for (span in urlSpans) { for (span in urlSpans) {
Assert.assertNotNull(mentions.firstOrNull { it.url == span.url }) assertNotNull(mentions.firstOrNull { it.url == span.url })
} }
} }
@ -72,7 +76,7 @@ class LinkHelperTest {
urlSpans = builder.getSpans(0, builder.length, URLSpan::class.java) urlSpans = builder.getSpans(0, builder.length, URLSpan::class.java)
for (span in urlSpans) { for (span in urlSpans) {
Assert.assertEquals(nonMentionUrl, span.url) assertEquals(nonMentionUrl, span.url)
} }
} }
@ -81,8 +85,8 @@ class LinkHelperTest {
for (tag in tags) { for (tag in tags) {
for (mutatedTagName in listOf(tag.name, tag.name.uppercase(), tag.name.lowercase())) { for (mutatedTagName in listOf(tag.name, tag.name.uppercase(), tag.name.lowercase())) {
val tagName = getTagName("#$mutatedTagName", tags) val tagName = getTagName("#$mutatedTagName", tags)
Assert.assertNotNull(tagName) assertNotNull(tagName)
Assert.assertNotNull(tags.firstOrNull { it.name == tagName }) assertNotNull(tags.firstOrNull { it.name == tagName })
} }
} }
} }
@ -93,22 +97,22 @@ class LinkHelperTest {
for (tag in tags) { for (tag in tags) {
val mutatedTagName = String(tag.name.map { mutator[it] ?: it }.toCharArray()) val mutatedTagName = String(tag.name.map { mutator[it] ?: it }.toCharArray())
val tagName = getTagName("#$mutatedTagName", tags) val tagName = getTagName("#$mutatedTagName", tags)
Assert.assertNotNull(tagName) assertNotNull(tagName)
Assert.assertNotNull(tags.firstOrNull { it.name == tagName }) assertNotNull(tags.firstOrNull { it.name == tagName })
} }
} }
@Test @Test
fun hashedUrlSpans_withNoMatchingTag_areNotModified() { fun hashedUrlSpans_withNoMatchingTag_areNotModified() {
for (tag in tags) { for (tag in tags) {
Assert.assertNull(getTagName("#not${tag.name}", tags)) assertNull(getTagName("#not${tag.name}", tags))
} }
} }
@Test @Test
fun whenTagsAreNull_tagNameIsGeneratedFromText() { fun whenTagsAreNull_tagNameIsGeneratedFromText() {
for (tag in tags) { for (tag in tags) {
Assert.assertEquals(tag.name, getTagName("#${tag.name}", null)) assertEquals(tag.name, getTagName("#${tag.name}", null))
} }
} }
@ -120,7 +124,7 @@ class LinkHelperTest {
"http:/foo.bar", "http:/foo.bar",
"c:/foo/bar" "c:/foo/bar"
).forEach { ).forEach {
Assert.assertEquals("", getDomain(it)) assertEquals("", getDomain(it))
} }
} }
@ -142,7 +146,7 @@ class LinkHelperTest {
"https://$domain/foo/bar.html?argument=value", "https://$domain/foo/bar.html?argument=value",
"https://$domain/foo/bar.html?argument=value&otherArgument=otherValue" "https://$domain/foo/bar.html?argument=value&otherArgument=otherValue"
).forEach { url -> ).forEach { url ->
Assert.assertEquals(domain, getDomain(url)) assertEquals(domain, getDomain(url))
} }
} }
} }
@ -155,7 +159,7 @@ class LinkHelperTest {
"http://www.localhost" to "localhost", "http://www.localhost" to "localhost",
"https://wwwexample.com/" to "wwwexample.com" "https://wwwexample.com/" to "wwwexample.com"
).forEach { (url, domain) -> ).forEach { (url, domain) ->
Assert.assertEquals(domain, getDomain(url)) assertEquals(domain, getDomain(url))
} }
} }
@ -167,11 +171,11 @@ class LinkHelperTest {
val content = SpannableStringBuilder() val content = SpannableStringBuilder()
content.append(displayedContent, URLSpan(maliciousUrl), 0) content.append(displayedContent, URLSpan(maliciousUrl), 0)
val oldContent = content.toString() val oldContent = content.toString()
Assert.assertEquals( assertEquals(
textView.context.getString(R.string.url_domain_notifier, displayedContent, maliciousDomain), displayedContent + " " + textView.context.getString(R.string.url_domain_notifier, maliciousDomain),
markupHiddenUrls(textView, content).toString() markupHiddenUrls(textView, content).toString()
) )
Assert.assertEquals(oldContent, content.toString()) assertEquals(oldContent, content.toString())
} }
@Test @Test
@ -181,8 +185,8 @@ class LinkHelperTest {
val maliciousUrl = "https://$maliciousDomain/to/go" val maliciousUrl = "https://$maliciousDomain/to/go"
val content = SpannableStringBuilder() val content = SpannableStringBuilder()
content.append(displayedContent, URLSpan(maliciousUrl), 0) content.append(displayedContent, URLSpan(maliciousUrl), 0)
Assert.assertEquals( assertEquals(
textView.context.getString(R.string.url_domain_notifier, displayedContent, maliciousDomain), displayedContent + " " + textView.context.getString(R.string.url_domain_notifier, maliciousDomain),
markupHiddenUrls(textView, content).toString() markupHiddenUrls(textView, content).toString()
) )
} }
@ -198,7 +202,7 @@ class LinkHelperTest {
val markedUpContent = markupHiddenUrls(textView, content) val markedUpContent = markupHiddenUrls(textView, content)
for (domain in domains) { for (domain in domains) {
Assert.assertTrue(markedUpContent.contains(textView.context.getString(R.string.url_domain_notifier, displayedContent, domain))) assertTrue(markedUpContent.contains(displayedContent + " " + textView.context.getString(R.string.url_domain_notifier, domain)))
} }
} }
@ -216,7 +220,7 @@ class LinkHelperTest {
.append("$domain/", URLSpan("https://www.$domain"), 0) .append("$domain/", URLSpan("https://www.$domain"), 0)
val markedUpContent = markupHiddenUrls(textView, content) val markedUpContent = markupHiddenUrls(textView, content)
Assert.assertFalse(markedUpContent.contains("🔗")) assertFalse(markedUpContent.contains("🔗"))
} }
@Test @Test
@ -229,7 +233,7 @@ class LinkHelperTest {
.append("Some Place https://some.place/path", URLSpan("https://some.place/path"), 0) .append("Some Place https://some.place/path", URLSpan("https://some.place/path"), 0)
val markedUpContent = markupHiddenUrls(textView, content) val markedUpContent = markupHiddenUrls(textView, content)
Assert.assertFalse(markedUpContent.contains("🔗")) assertFalse(markedUpContent.contains("🔗"))
} }
@Test @Test
@ -249,8 +253,9 @@ class LinkHelperTest {
"Another Place | https://another.place/", "Another Place | https://another.place/",
"Another Place https://another.place/path" "Another Place https://another.place/path"
) )
print(markedUpContent)
asserts.forEach { asserts.forEach {
Assert.assertTrue(markedUpContent.contains(textView.context.getString(R.string.url_domain_notifier, it, "some.place"))) assertTrue(markedUpContent.contains(it + " " + textView.context.getString(R.string.url_domain_notifier, "some.place")))
} }
} }
@ -264,7 +269,7 @@ class LinkHelperTest {
val markedUpContent = markupHiddenUrls(textView, builder) val markedUpContent = markupHiddenUrls(textView, builder)
for (mention in mentions) { for (mention in mentions) {
Assert.assertFalse(markedUpContent.contains("${getDomain(mention.url)})")) assertFalse(markedUpContent.contains("${getDomain(mention.url)})"))
} }
} }
@ -278,7 +283,7 @@ class LinkHelperTest {
val markedUpContent = markupHiddenUrls(textView, builder) val markedUpContent = markupHiddenUrls(textView, builder)
for (mention in mentions) { for (mention in mentions) {
Assert.assertFalse(markedUpContent.contains("${getDomain(mention.url)})")) assertFalse(markedUpContent.contains("${getDomain(mention.url)})"))
} }
} }
@ -292,7 +297,7 @@ class LinkHelperTest {
val markedUpContent = markupHiddenUrls(textView, builder) val markedUpContent = markupHiddenUrls(textView, builder)
for (tag in tags) { for (tag in tags) {
Assert.assertFalse(markedUpContent.contains("${getDomain(tag.url)})")) assertFalse(markedUpContent.contains("${getDomain(tag.url)})"))
} }
} }
@ -306,7 +311,7 @@ class LinkHelperTest {
val markedUpContent = markupHiddenUrls(textView, builder) val markedUpContent = markupHiddenUrls(textView, builder)
for (tag in tags) { for (tag in tags) {
Assert.assertFalse(markedUpContent.contains("${getDomain(tag.url)})")) assertFalse(markedUpContent.contains("${getDomain(tag.url)})"))
} }
} }
@ -375,7 +380,7 @@ class LinkHelperTest {
@Test @Test
fun test() { fun test() {
Assert.assertEquals(expectedResult, looksLikeMastodonUrl(url)) assertEquals(expectedResult, looksLikeMastodonUrl(url))
} }
} }
} }