Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix how emojis in link texts are displayed #4723

Merged
merged 2 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions app/src/main/java/com/keylesspalace/tusky/util/LinkHelper.kt
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
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
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))
}
}
}