Skip to content

Commit

Permalink
fix how emojis in link texts are displayed (#4723)
Browse files Browse the repository at this point in the history 
![Screenshot_20241010_204357](https://github.com/user-attachments/assets/6f511231-b89c-4902-ad89-68c1940415a7)

closes #4720
  • Loading branch information
@connyduck
connyduck authored Oct 11, 2024
1 parent 0bc227b commit f8cf38c
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 37 deletions.
19 changes: 9 additions & 10 deletions app/src/main/java/com/keylesspalace/tusky/util/LinkHelper.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
}
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/res/values/donottranslate.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,6 @@
<item>NEWEST_FIRST</item>
</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>
57 changes: 31 additions & 26 deletions app/src/test/java/com/keylesspalace/tusky/util/LinkHelperTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 })
}
}

Expand All @@ -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)
}
}

Expand All @@ -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 })
}
}
}
Expand All @@ -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))
}
}

Expand All @@ -120,7 +124,7 @@ class LinkHelperTest {
"http:/foo.bar",
"c:/foo/bar"
).forEach {
Assert.assertEquals("", getDomain(it))
assertEquals("", getDomain(it))
}
}

Expand All @@ -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))
}
}
}
Expand All @@ -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))
}
}

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

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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")))
}
}

Expand All @@ -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)})"))
}
}

Expand All @@ -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)})"))
}
}

Expand All @@ -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)})"))
}
}

Expand All @@ -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)})"))
}
}

Expand Down Expand Up @@ -375,7 +380,7 @@ class LinkHelperTest {

@Test
fun test() {
Assert.assertEquals(expectedResult, looksLikeMastodonUrl(url))
assertEquals(expectedResult, looksLikeMastodonUrl(url))
}
}
}

0 comments on commit f8cf38c

Please sign in to comment.