From 6dfdaec425c3013c3299c56dc4d676b70677efb8 Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Mon, 13 Mar 2023 10:22:33 +0100 Subject: [PATCH] Ignore "@instance..." part of username when computing status length (#3392) * Move compose.* tests to own namespace * Ignore "@instance..." part of username when computing status length In a status with a mention ("@foo@example.org") only the "@foo" part should be included in the calculated status length. It wasn't, so the app was prevening people from posting statuses that should have been allowed. Fix this. - Lift the length calculation code in to a separate static function (easier and faster to test) - Add a `MentionSpan` type, to reuse existing code for detecting mentions - Fix a bug in `FakeSpannable.getSpans()` (it was returning the outer type, not the wrapped inner span) - Add additional fast tests The tests made sense under the `components.compose.ComposeActivity` package, so I also created that and moved the existing ComposeActivity tests there. Fixes https://github.com/tuskyapp/Tusky/issues/3339 * Static import assertEquals --- .../components/compose/ComposeActivity.kt | 70 ++++++++++++---- .../keylesspalace/tusky/util/LinkHelper.kt | 2 +- .../tusky/util/NoUnderlineURLSpan.kt | 16 +++- .../com/keylesspalace/tusky/util/SpanUtils.kt | 1 + .../com/keylesspalace/tusky/SpanUtilsTest.kt | 2 +- .../ComposeActivity}/ComposeActivityTest.kt | 9 ++- .../ComposeActivity/StatusLengthTest.kt | 79 +++++++++++++++++++ .../ComposeTokenizer}/ComposeTokenizerTest.kt | 2 +- 8 files changed, 158 insertions(+), 23 deletions(-) rename app/src/test/java/com/keylesspalace/tusky/{ => components/compose/ComposeActivity}/ComposeActivityTest.kt (99%) create mode 100644 app/src/test/java/com/keylesspalace/tusky/components/compose/ComposeActivity/StatusLengthTest.kt rename app/src/test/java/com/keylesspalace/tusky/{ => components/compose/ComposeTokenizer}/ComposeTokenizerTest.kt (98%) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt index 55ea6b155..daacc711d 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt @@ -31,6 +31,8 @@ import android.os.Build import android.os.Bundle import android.os.Parcelable import android.provider.MediaStore +import android.text.Spanned +import android.text.style.URLSpan import android.util.Log import android.view.KeyEvent import android.view.MenuItem @@ -91,6 +93,7 @@ import com.keylesspalace.tusky.entity.NewPoll import com.keylesspalace.tusky.entity.Status import com.keylesspalace.tusky.settings.PrefKeys import com.keylesspalace.tusky.util.APP_THEME_DEFAULT +import com.keylesspalace.tusky.util.MentionSpan import com.keylesspalace.tusky.util.PickMediaFiles import com.keylesspalace.tusky.util.getInitialLanguages import com.keylesspalace.tusky.util.getLocaleList @@ -883,20 +886,11 @@ class ComposeActivity : @VisibleForTesting fun calculateTextLength(): Int { - var offset = 0 - val urlSpans = binding.composeEditField.urls - if (urlSpans != null) { - for (span in urlSpans) { - // it's expected that this will be negative - // when the url length is less than the reserved character count - offset += (span.url.length - charactersReservedPerUrl) - } - } - var length = binding.composeEditField.length() - offset - if (viewModel.showContentWarning.value) { - length += binding.composeContentWarningField.length() - } - return length + return statusLength( + binding.composeEditField.text, + binding.composeContentWarningField.text, + charactersReservedPerUrl + ) } @VisibleForTesting @@ -1352,5 +1346,53 @@ class ComposeActivity : fun canHandleMimeType(mimeType: String?): Boolean { return mimeType != null && (mimeType.startsWith("image/") || mimeType.startsWith("video/") || mimeType.startsWith("audio/") || mimeType == "text/plain") } + + /** + * Calculate the effective status length. + * + * Some text is counted differently: + * + * In the status body: + * + * - URLs always count for [urlLength] characters irrespective of their actual length + * (https://docs.joinmastodon.org/user/posting/#links) + * - Mentions ("@user@some.instance") only count the "@user" part + * (https://docs.joinmastodon.org/user/posting/#mentions) + * - Hashtags are always treated as their actual length, including the "#" + * (https://docs.joinmastodon.org/user/posting/#hashtags) + * + * Content warning text is always treated as its full length, URLs and other entities + * are not treated differently. + * + * @param body status body text + * @param contentWarning optional content warning text + * @param urlLength the number of characters attributed to URLs + * @return the effective status length + */ + @JvmStatic + fun statusLength(body: Spanned, contentWarning: Spanned?, urlLength: Int): Int { + var length = body.length - body.getSpans(0, body.length, URLSpan::class.java) + .fold(0) { acc, span -> + // Accumulate a count of characters to be *ignored* in the final length + acc + when (span) { + is MentionSpan -> { + // Ignore everything from the second "@" (if present) + span.url.length - ( + span.url.indexOf("@", 1).takeIf { it >= 0 } + ?: span.url.length + ) + } + else -> { + // Expected to be negative if the URL length < maxUrlLength + span.url.length - urlLength + } + } + } + + // Content warning text is treated as is, URLs or mentions there are not special + contentWarning?.let { length += it.length } + + return length + } } } 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 0f6d502e1..04176a11e 100644 --- a/app/src/main/java/com/keylesspalace/tusky/util/LinkHelper.kt +++ b/app/src/main/java/com/keylesspalace/tusky/util/LinkHelper.kt @@ -156,7 +156,7 @@ private fun getCustomSpanForMention(mentions: List, span: URLSpan, list } private fun getCustomSpanForMentionUrl(url: String, mentionId: String, listener: LinkListener): ClickableSpan { - return object : NoUnderlineURLSpan(url) { + return object : MentionSpan(url) { override fun onClick(view: View) = listener.onViewAccount(mentionId) } } diff --git a/app/src/main/java/com/keylesspalace/tusky/util/NoUnderlineURLSpan.kt b/app/src/main/java/com/keylesspalace/tusky/util/NoUnderlineURLSpan.kt index 779a7e6e9..12be84d8a 100644 --- a/app/src/main/java/com/keylesspalace/tusky/util/NoUnderlineURLSpan.kt +++ b/app/src/main/java/com/keylesspalace/tusky/util/NoUnderlineURLSpan.kt @@ -19,9 +19,14 @@ import android.text.TextPaint import android.text.style.URLSpan import android.view.View -open class NoUnderlineURLSpan( - url: String -) : URLSpan(url) { +open class NoUnderlineURLSpan constructor(val url: String) : URLSpan(url) { + + // This should not be necessary. But if you don't do this the [StatusLengthTest] tests + // fail. Without this, accessing the `url` property, or calling `getUrl()` (which should + // automatically call through to [UrlSpan.getURL]) returns null. + override fun getURL(): String { + return url + } override fun updateDrawState(ds: TextPaint) { super.updateDrawState(ds) @@ -32,3 +37,8 @@ open class NoUnderlineURLSpan( view.context.openLink(url) } } + +/** + * Mentions of other users ("@user@example.org") + */ +open class MentionSpan(url: String) : NoUnderlineURLSpan(url) diff --git a/app/src/main/java/com/keylesspalace/tusky/util/SpanUtils.kt b/app/src/main/java/com/keylesspalace/tusky/util/SpanUtils.kt index 670b02edd..9bb3b41a9 100644 --- a/app/src/main/java/com/keylesspalace/tusky/util/SpanUtils.kt +++ b/app/src/main/java/com/keylesspalace/tusky/util/SpanUtils.kt @@ -131,6 +131,7 @@ private fun getSpan(matchType: FoundMatchType, string: String, colour: Int, star return when (matchType) { FoundMatchType.HTTP_URL -> NoUnderlineURLSpan(string.substring(start, end)) FoundMatchType.HTTPS_URL -> NoUnderlineURLSpan(string.substring(start, end)) + FoundMatchType.MENTION -> MentionSpan(string.substring(start, end)) else -> ForegroundColorSpan(colour) } } diff --git a/app/src/test/java/com/keylesspalace/tusky/SpanUtilsTest.kt b/app/src/test/java/com/keylesspalace/tusky/SpanUtilsTest.kt index 268ae5d36..45e6187a5 100644 --- a/app/src/test/java/com/keylesspalace/tusky/SpanUtilsTest.kt +++ b/app/src/test/java/com/keylesspalace/tusky/SpanUtilsTest.kt @@ -136,7 +136,7 @@ class SpanUtilsTest { } override fun getSpans(start: Int, end: Int, type: Class): Array { - return spans.filter { it.start >= start && it.end <= end && type.isInstance(it) } + return spans.filter { it.start >= start && it.end <= end && type.isInstance(it.span) } .map { it.span } .toTypedArray() as Array } diff --git a/app/src/test/java/com/keylesspalace/tusky/ComposeActivityTest.kt b/app/src/test/java/com/keylesspalace/tusky/components/compose/ComposeActivity/ComposeActivityTest.kt similarity index 99% rename from app/src/test/java/com/keylesspalace/tusky/ComposeActivityTest.kt rename to app/src/test/java/com/keylesspalace/tusky/components/compose/ComposeActivity/ComposeActivityTest.kt index 403f25edc..f16355b57 100644 --- a/app/src/test/java/com/keylesspalace/tusky/ComposeActivityTest.kt +++ b/app/src/test/java/com/keylesspalace/tusky/components/compose/ComposeActivity/ComposeActivityTest.kt @@ -1,4 +1,5 @@ -/* Copyright 2018 charlag +/* + * Copyright 2018 Tusky Contributors * * This file is a part of Tusky. * @@ -11,15 +12,17 @@ * Public License for more details. * * You should have received a copy of the GNU General Public License along with Tusky; if not, - * see . */ + * see . + */ -package com.keylesspalace.tusky +package com.keylesspalace.tusky.components.compose.ComposeActivity import android.content.Intent import android.os.Looper.getMainLooper import android.widget.EditText import androidx.test.ext.junit.runners.AndroidJUnit4 import at.connyduck.calladapter.networkresult.NetworkResult +import com.keylesspalace.tusky.R import com.keylesspalace.tusky.components.compose.ComposeActivity import com.keylesspalace.tusky.components.compose.ComposeViewModel import com.keylesspalace.tusky.components.instanceinfo.InstanceInfoRepository diff --git a/app/src/test/java/com/keylesspalace/tusky/components/compose/ComposeActivity/StatusLengthTest.kt b/app/src/test/java/com/keylesspalace/tusky/components/compose/ComposeActivity/StatusLengthTest.kt new file mode 100644 index 000000000..81c40a556 --- /dev/null +++ b/app/src/test/java/com/keylesspalace/tusky/components/compose/ComposeActivity/StatusLengthTest.kt @@ -0,0 +1,79 @@ +/* + * Copyright 2023 Tusky Contributors + * + * This file is a part of Tusky. + * + * This program is free software; you can redistribute it and/or modify it under the terms of the + * GNU General Public License as published by the Free Software Foundation; either version 3 of the + * License, or (at your option) any later version. + * + * Tusky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even + * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General + * Public License for more details. + * + * You should have received a copy of the GNU General Public License along with Tusky; if not, + * see . + */ + +package com.keylesspalace.tusky.components.compose.ComposeActivity + +import com.keylesspalace.tusky.SpanUtilsTest +import com.keylesspalace.tusky.components.compose.ComposeActivity +import com.keylesspalace.tusky.util.highlightSpans +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized + +@RunWith(Parameterized::class) +class StatusLengthTest( + private val text: String, + private val expectedLength: Int +) { + companion object { + @Parameterized.Parameters(name = "{0}") + @JvmStatic + fun data(): Iterable { + return listOf( + arrayOf("", 0), + arrayOf(" ", 1), + arrayOf("123", 3), + // "@user@server" should be treated as "@user" + arrayOf("123 @example@example.org", 12), + // URLs under 23 chars are treated as 23 chars + arrayOf("123 http://example.url", 27), + // URLs over 23 chars are treated as 23 chars + arrayOf("123 http://urlthatislongerthan23characters.example.org", 27), + // Short hashtags are treated as is + arrayOf("123 #basictag", 13), + // Long hashtags are *also* treated as is (not treated as 23, like URLs) + arrayOf("123 #atagthatislongerthan23characters", 37) + ) + } + } + + @Test + fun statusLength_matchesExpectations() { + val spannedText = SpanUtilsTest.FakeSpannable(text) + highlightSpans(spannedText, 0) + + assertEquals( + expectedLength, + ComposeActivity.statusLength(spannedText, null, 23) + ) + } + + @Test + fun statusLength_withCwText_matchesExpectations() { + val spannedText = SpanUtilsTest.FakeSpannable(text) + highlightSpans(spannedText, 0) + + val cwText = SpanUtilsTest.FakeSpannable( + "a @example@example.org #hashtagmention and http://example.org URL" + ) + assertEquals( + expectedLength + cwText.length, + ComposeActivity.statusLength(spannedText, cwText, 23) + ) + } +} diff --git a/app/src/test/java/com/keylesspalace/tusky/ComposeTokenizerTest.kt b/app/src/test/java/com/keylesspalace/tusky/components/compose/ComposeTokenizer/ComposeTokenizerTest.kt similarity index 98% rename from app/src/test/java/com/keylesspalace/tusky/ComposeTokenizerTest.kt rename to app/src/test/java/com/keylesspalace/tusky/components/compose/ComposeTokenizer/ComposeTokenizerTest.kt index fa0bba94e..04c692bce 100644 --- a/app/src/test/java/com/keylesspalace/tusky/ComposeTokenizerTest.kt +++ b/app/src/test/java/com/keylesspalace/tusky/components/compose/ComposeTokenizer/ComposeTokenizerTest.kt @@ -13,7 +13,7 @@ * You should have received a copy of the GNU General Public License along with Tusky; if not, * see . */ -package com.keylesspalace.tusky +package com.keylesspalace.tusky.components.compose.ComposeTokenizer import com.keylesspalace.tusky.components.compose.ComposeTokenizer import org.junit.Assert