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
This commit is contained in:
Levi Bard 2022-11-01 16:41:55 +01:00 committed by GitHub
parent 5887af3772
commit 58e8f75287
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 45 additions and 13 deletions

View File

@ -57,7 +57,7 @@ fun getDomain(urlString: String?): String {
* @param listener to notify about particular spans that are clicked * @param listener to notify about particular spans that are clicked
*/ */
fun setClickableText(view: TextView, content: CharSequence, mentions: List<Mention>, tags: List<HashTag>?, listener: LinkListener) { fun setClickableText(view: TextView, content: CharSequence, mentions: List<Mention>, tags: List<HashTag>?, listener: LinkListener) {
val spannableContent = markupHiddenUrls(view.context, content, mentions, tags, listener) val spannableContent = markupHiddenUrls(view.context, content)
view.text = spannableContent.apply { view.text = spannableContent.apply {
getSpans(0, content.length, URLSpan::class.java).forEach { getSpans(0, content.length, URLSpan::class.java).forEach {
@ -68,15 +68,26 @@ fun setClickableText(view: TextView, content: CharSequence, mentions: List<Menti
} }
@VisibleForTesting @VisibleForTesting
fun markupHiddenUrls(context: Context, content: CharSequence, mentions: List<Mention>, tags: List<HashTag>?, listener: LinkListener): SpannableStringBuilder { fun markupHiddenUrls(context: Context, content: CharSequence): SpannableStringBuilder {
val spannableContent = SpannableStringBuilder.valueOf(content) val spannableContent = SpannableStringBuilder.valueOf(content)
val originalSpans = spannableContent.getSpans(0, content.length, URLSpan::class.java) val originalSpans = spannableContent.getSpans(0, content.length, URLSpan::class.java)
val obscuredLinkSpans = originalSpans.filter { val obscuredLinkSpans = originalSpans.filter {
val text = spannableContent.subSequence(spannableContent.getSpanStart(it), spannableContent.getSpanEnd(it)) val text = spannableContent.subSequence(spannableContent.getSpanStart(it), spannableContent.getSpanEnd(it))
val firstCharacter = text[0] val firstCharacter = text[0]
firstCharacter != '#' && return@filter if (firstCharacter == '#' || firstCharacter == '@') {
firstCharacter != '@' && false
getDomain(text.toString()) != getDomain(it.url) } 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) { for (span in obscuredLinkSpans) {

View File

@ -165,7 +165,10 @@ 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(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 @Test
@ -175,10 +178,14 @@ 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(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 domains = listOf("one.place", "another.place", "athird.place")
val displayedContent = "link" val displayedContent = "link"
val content = SpannableStringBuilder() val content = SpannableStringBuilder()
@ -186,12 +193,26 @@ class LinkHelperTest {
content.append(displayedContent, URLSpan("https://$domain/foo/bar"), 0) 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) { for (domain in domains) {
Assert.assertTrue(markedUpContent.contains(context.getString(R.string.url_domain_notifier, displayedContent, domain))) 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 @Test
fun validMentionsAreNotMarkedUp() { fun validMentionsAreNotMarkedUp() {
val builder = SpannableStringBuilder() val builder = SpannableStringBuilder()
@ -200,7 +221,7 @@ class LinkHelperTest {
builder.append(" ") builder.append(" ")
} }
val markedUpContent = markupHiddenUrls(context, builder, mentions, tags, listener) val markedUpContent = markupHiddenUrls(context, builder)
for (mention in mentions) { for (mention in mentions) {
Assert.assertFalse(markedUpContent.contains("${getDomain(mention.url)})")) Assert.assertFalse(markedUpContent.contains("${getDomain(mention.url)})"))
} }
@ -214,7 +235,7 @@ class LinkHelperTest {
builder.append(" ") builder.append(" ")
} }
val markedUpContent = markupHiddenUrls(context, builder, listOf(), tags, listener) val markedUpContent = markupHiddenUrls(context, builder)
for (mention in mentions) { for (mention in mentions) {
Assert.assertFalse(markedUpContent.contains("${getDomain(mention.url)})")) Assert.assertFalse(markedUpContent.contains("${getDomain(mention.url)})"))
} }
@ -228,7 +249,7 @@ class LinkHelperTest {
builder.append(" ") builder.append(" ")
} }
val markedUpContent = markupHiddenUrls(context, builder, mentions, tags, listener) val markedUpContent = markupHiddenUrls(context, builder)
for (tag in tags) { for (tag in tags) {
Assert.assertFalse(markedUpContent.contains("${getDomain(tag.url)})")) Assert.assertFalse(markedUpContent.contains("${getDomain(tag.url)})"))
} }
@ -242,7 +263,7 @@ class LinkHelperTest {
builder.append(" ") builder.append(" ")
} }
val markedUpContent = markupHiddenUrls(context, builder, mentions, listOf(), listener) val markedUpContent = markupHiddenUrls(context, builder)
for (tag in tags) { for (tag in tags) {
Assert.assertFalse(markedUpContent.contains("${getDomain(tag.url)})")) Assert.assertFalse(markedUpContent.contains("${getDomain(tag.url)})"))
} }