From 58e8f75287652b08f42f4171b1264cb96992c319 Mon Sep 17 00:00:00 2001 From: Levi Bard Date: Tue, 1 Nov 2022 16:41:55 +0100 Subject: [PATCH] Don't markup urls where the display text is exactly the domain (#2732) * Don't markup urls where the display text is exactly the domain (or www.thedomain) * Remove unused arguments --- .../keylesspalace/tusky/util/LinkHelper.kt | 21 ++++++++--- .../tusky/util/LinkHelperTest.kt | 37 +++++++++++++++---- 2 files changed, 45 insertions(+), 13 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 9a25ecaa..6a5b9d54 100644 --- a/app/src/main/java/com/keylesspalace/tusky/util/LinkHelper.kt +++ b/app/src/main/java/com/keylesspalace/tusky/util/LinkHelper.kt @@ -57,7 +57,7 @@ fun getDomain(urlString: String?): String { * @param listener to notify about particular spans that are clicked */ fun setClickableText(view: TextView, content: CharSequence, mentions: List, tags: List?, listener: LinkListener) { - val spannableContent = markupHiddenUrls(view.context, content, mentions, tags, listener) + val spannableContent = markupHiddenUrls(view.context, content) view.text = spannableContent.apply { getSpans(0, content.length, URLSpan::class.java).forEach { @@ -68,15 +68,26 @@ fun setClickableText(view: TextView, content: CharSequence, mentions: List, tags: List?, listener: LinkListener): SpannableStringBuilder { +fun markupHiddenUrls(context: Context, content: CharSequence): SpannableStringBuilder { val spannableContent = SpannableStringBuilder.valueOf(content) val originalSpans = spannableContent.getSpans(0, content.length, URLSpan::class.java) val obscuredLinkSpans = originalSpans.filter { val text = spannableContent.subSequence(spannableContent.getSpanStart(it), spannableContent.getSpanEnd(it)) val firstCharacter = text[0] - firstCharacter != '#' && - firstCharacter != '@' && - getDomain(text.toString()) != getDomain(it.url) + return@filter if (firstCharacter == '#' || firstCharacter == '@') { + false + } else { + var textDomain = getDomain(text.toString()) + if (textDomain.isBlank()) { + // Allow "some.domain" or "www.some.domain" without a domain notifier + textDomain = if (text.startsWith("www.")) { + text.substring(4) + } else { + text.toString() + } + } + getDomain(it.url) != textDomain + } } for (span in obscuredLinkSpans) { 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 9e8f899e..d7a61ccb 100644 --- a/app/src/test/java/com/keylesspalace/tusky/util/LinkHelperTest.kt +++ b/app/src/test/java/com/keylesspalace/tusky/util/LinkHelperTest.kt @@ -165,7 +165,10 @@ class LinkHelperTest { val maliciousUrl = "https://$maliciousDomain/to/go" val content = SpannableStringBuilder() content.append(displayedContent, URLSpan(maliciousUrl), 0) - Assert.assertEquals(context.getString(R.string.url_domain_notifier, displayedContent, maliciousDomain), markupHiddenUrls(context, content, listOf(), listOf(), listener).toString()) + Assert.assertEquals( + context.getString(R.string.url_domain_notifier, displayedContent, maliciousDomain), + markupHiddenUrls(context, content).toString() + ) } @Test @@ -175,10 +178,14 @@ class LinkHelperTest { val maliciousUrl = "https://$maliciousDomain/to/go" val content = SpannableStringBuilder() content.append(displayedContent, URLSpan(maliciousUrl), 0) - Assert.assertEquals(context.getString(R.string.url_domain_notifier, displayedContent, maliciousDomain), markupHiddenUrls(context, content, listOf(), listOf(), listener).toString()) + Assert.assertEquals( + context.getString(R.string.url_domain_notifier, displayedContent, maliciousDomain), + markupHiddenUrls(context, content).toString() + ) } - @Test fun multipleHiddenDomainsAreMarkedUp() { + @Test + fun multipleHiddenDomainsAreMarkedUp() { val domains = listOf("one.place", "another.place", "athird.place") val displayedContent = "link" val content = SpannableStringBuilder() @@ -186,12 +193,26 @@ class LinkHelperTest { content.append(displayedContent, URLSpan("https://$domain/foo/bar"), 0) } - val markedUpContent = markupHiddenUrls(context, content, listOf(), listOf(), listener) + val markedUpContent = markupHiddenUrls(context, content) for (domain in domains) { Assert.assertTrue(markedUpContent.contains(context.getString(R.string.url_domain_notifier, displayedContent, domain))) } } + @Test + fun nonUriTextExactlyMatchingDomainIsNotMarkedUp() { + val domain = "some.place" + val content = SpannableStringBuilder() + .append(domain, URLSpan("https://some.place/"), 0) + .append(domain, URLSpan("https://some.place"), 0) + .append(domain, URLSpan("https://www.some.place"), 0) + .append("www.$domain", URLSpan("https://some.place"), 0) + .append("www.$domain", URLSpan("https://some.place/"), 0) + + val markedUpContent = markupHiddenUrls(context, content) + Assert.assertFalse(markedUpContent.contains("🔗")) + } + @Test fun validMentionsAreNotMarkedUp() { val builder = SpannableStringBuilder() @@ -200,7 +221,7 @@ class LinkHelperTest { builder.append(" ") } - val markedUpContent = markupHiddenUrls(context, builder, mentions, tags, listener) + val markedUpContent = markupHiddenUrls(context, builder) for (mention in mentions) { Assert.assertFalse(markedUpContent.contains("${getDomain(mention.url)})")) } @@ -214,7 +235,7 @@ class LinkHelperTest { builder.append(" ") } - val markedUpContent = markupHiddenUrls(context, builder, listOf(), tags, listener) + val markedUpContent = markupHiddenUrls(context, builder) for (mention in mentions) { Assert.assertFalse(markedUpContent.contains("${getDomain(mention.url)})")) } @@ -228,7 +249,7 @@ class LinkHelperTest { builder.append(" ") } - val markedUpContent = markupHiddenUrls(context, builder, mentions, tags, listener) + val markedUpContent = markupHiddenUrls(context, builder) for (tag in tags) { Assert.assertFalse(markedUpContent.contains("${getDomain(tag.url)})")) } @@ -242,7 +263,7 @@ class LinkHelperTest { builder.append(" ") } - val markedUpContent = markupHiddenUrls(context, builder, mentions, listOf(), listener) + val markedUpContent = markupHiddenUrls(context, builder) for (tag in tags) { Assert.assertFalse(markedUpContent.contains("${getDomain(tag.url)})")) }