From f8cf38c81b2a0dfa613daca09dfe4c67b430ce1e Mon Sep 17 00:00:00 2001 From: Konrad Pozniak Date: Fri, 11 Oct 2024 11:25:43 +0200 Subject: [PATCH] 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 --- .../keylesspalace/tusky/util/LinkHelper.kt | 19 +++---- app/src/main/res/values/donottranslate.xml | 2 +- .../tusky/util/LinkHelperTest.kt | 57 ++++++++++--------- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/util/LinkHelper.kt b/app/src/main/java/com/keylesspalace/tusky/util/LinkHelper.kt index ed790de3dc..7d83857c36 100644 --- a/app/src/main/java/com/keylesspalace/tusky/util/LinkHelper.kt +++ b/app/src/main/java/com/keylesspalace/tusky/util/LinkHelper.kt @@ -115,29 +115,28 @@ fun markupHiddenUrls(view: TextView, content: CharSequence): SpannableStringBuil for (span in obscuredLinkSpans) { val start = spannableContent.getSpanStart(span) val end = spannableContent.getSpanEnd(span) - val originalText = spannableContent.subSequence(start, end) - val replacementText = view.context.getString( + val additionalText = " " + view.context.getString( R.string.url_domain_notifier, - originalText, getDomain(span.url) ) - spannableContent.replace( - start, + spannableContent.insert( end, - replacementText - ) // this also updates the span locations + additionalText + ) + // 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)!! // ImageSpan does not always align the icon correctly in the line, let's use our custom emoji span for this val linkDrawableSpan = EmojiSpan(view) linkDrawableSpan.imageDrawable = linkDrawable - val placeholderIndex = originalText.length + 2 + val placeholderIndex = end + 2 spannableContent.setSpan( linkDrawableSpan, - start + placeholderIndex, - start + placeholderIndex + "🔗".length, + placeholderIndex, + placeholderIndex + "🔗".length, 0 ) } diff --git a/app/src/main/res/values/donottranslate.xml b/app/src/main/res/values/donottranslate.xml index 3484db0ca3..17dc3c9d7b 100644 --- a/app/src/main/res/values/donottranslate.xml +++ b/app/src/main/res/values/donottranslate.xml @@ -265,6 +265,6 @@ NEWEST_FIRST - %1$s (🔗 %2$s) + (🔗 %1$s) diff --git a/app/src/test/java/com/keylesspalace/tusky/util/LinkHelperTest.kt b/app/src/test/java/com/keylesspalace/tusky/util/LinkHelperTest.kt index 99be370c5c..c4ba4c4640 100644 --- a/app/src/test/java/com/keylesspalace/tusky/util/LinkHelperTest.kt +++ b/app/src/test/java/com/keylesspalace/tusky/util/LinkHelperTest.kt @@ -9,7 +9,11 @@ import com.keylesspalace.tusky.R import com.keylesspalace.tusky.entity.HashTag import com.keylesspalace.tusky.entity.Status 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.runner.RunWith import org.junit.runners.Parameterized @@ -51,7 +55,7 @@ class LinkHelperTest { urlSpans = builder.getSpans(0, builder.length, URLSpan::class.java) 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) for (span in urlSpans) { - Assert.assertEquals(nonMentionUrl, span.url) + assertEquals(nonMentionUrl, span.url) } } @@ -81,8 +85,8 @@ class LinkHelperTest { for (tag in tags) { for (mutatedTagName in listOf(tag.name, tag.name.uppercase(), tag.name.lowercase())) { val tagName = getTagName("#$mutatedTagName", tags) - Assert.assertNotNull(tagName) - Assert.assertNotNull(tags.firstOrNull { it.name == tagName }) + assertNotNull(tagName) + assertNotNull(tags.firstOrNull { it.name == tagName }) } } } @@ -93,22 +97,22 @@ class LinkHelperTest { for (tag in tags) { val mutatedTagName = String(tag.name.map { mutator[it] ?: it }.toCharArray()) val tagName = getTagName("#$mutatedTagName", tags) - Assert.assertNotNull(tagName) - Assert.assertNotNull(tags.firstOrNull { it.name == tagName }) + assertNotNull(tagName) + assertNotNull(tags.firstOrNull { it.name == tagName }) } } @Test fun hashedUrlSpans_withNoMatchingTag_areNotModified() { for (tag in tags) { - Assert.assertNull(getTagName("#not${tag.name}", tags)) + assertNull(getTagName("#not${tag.name}", tags)) } } @Test fun whenTagsAreNull_tagNameIsGeneratedFromText() { 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", "c:/foo/bar" ).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&otherArgument=otherValue" ).forEach { url -> - Assert.assertEquals(domain, getDomain(url)) + assertEquals(domain, getDomain(url)) } } } @@ -155,7 +159,7 @@ class LinkHelperTest { "http://www.localhost" to "localhost", "https://wwwexample.com/" to "wwwexample.com" ).forEach { (url, domain) -> - Assert.assertEquals(domain, getDomain(url)) + assertEquals(domain, getDomain(url)) } } @@ -167,11 +171,11 @@ class LinkHelperTest { val content = SpannableStringBuilder() content.append(displayedContent, URLSpan(maliciousUrl), 0) val oldContent = content.toString() - Assert.assertEquals( - textView.context.getString(R.string.url_domain_notifier, displayedContent, maliciousDomain), + assertEquals( + displayedContent + " " + textView.context.getString(R.string.url_domain_notifier, maliciousDomain), markupHiddenUrls(textView, content).toString() ) - Assert.assertEquals(oldContent, content.toString()) + assertEquals(oldContent, content.toString()) } @Test @@ -181,8 +185,8 @@ class LinkHelperTest { val maliciousUrl = "https://$maliciousDomain/to/go" val content = SpannableStringBuilder() content.append(displayedContent, URLSpan(maliciousUrl), 0) - Assert.assertEquals( - textView.context.getString(R.string.url_domain_notifier, displayedContent, maliciousDomain), + assertEquals( + displayedContent + " " + textView.context.getString(R.string.url_domain_notifier, maliciousDomain), markupHiddenUrls(textView, content).toString() ) } @@ -198,7 +202,7 @@ class LinkHelperTest { val markedUpContent = markupHiddenUrls(textView, content) 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) val markedUpContent = markupHiddenUrls(textView, content) - Assert.assertFalse(markedUpContent.contains("🔗")) + assertFalse(markedUpContent.contains("🔗")) } @Test @@ -229,7 +233,7 @@ class LinkHelperTest { .append("Some Place https://some.place/path", URLSpan("https://some.place/path"), 0) val markedUpContent = markupHiddenUrls(textView, content) - Assert.assertFalse(markedUpContent.contains("🔗")) + assertFalse(markedUpContent.contains("🔗")) } @Test @@ -249,8 +253,9 @@ class LinkHelperTest { "Another Place | https://another.place/", "Another Place https://another.place/path" ) + print(markedUpContent) 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) 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) 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) 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) for (tag in tags) { - Assert.assertFalse(markedUpContent.contains("${getDomain(tag.url)})")) + assertFalse(markedUpContent.contains("${getDomain(tag.url)})")) } } @@ -375,7 +380,7 @@ class LinkHelperTest { @Test fun test() { - Assert.assertEquals(expectedResult, looksLikeMastodonUrl(url)) + assertEquals(expectedResult, looksLikeMastodonUrl(url)) } } }