fix: Calculate length of posts and polls with emojis correctly (#315)

Mastodon counts post lengths by considering emojis to be single
characters, no matter how many unicode code points they are composed of.
So "😜" has length 1.

Pachli was using `String.length`, which considers "😜" as length 2.

Correct the calculation by using a BreakIterator to count the characters
in the string, which treats multi-character emojis as a length 1.

Poll options had a similar problem, exacerbated by the Mastodon web UI
also having the same problem, see
https://github.com/mastodon/mastodon/issues/28336.

Fix that by creating `MastodonLengthFilter`, an `InputFilter` that does
the right thing for regular text that may contain emojis.

See also https://github.com/tuskyapp/Tusky/pull/4152, which has the fix
for status length but not polls.

---------

Co-authored-by: Konrad Pozniak <opensource@connyduck.at>
This commit is contained in:
Nik Clayton 2023-12-12 16:53:09 +01:00 committed by GitHub
parent 239f5cb274
commit 098983f401
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 399 additions and 104 deletions

View File

@ -872,7 +872,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/app/pachli/components/account/AccountActivity.kt"
line="484"
line="492"
column="9"/>
</issue>
@ -2082,7 +2082,7 @@
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/components/account/AccountActivity.kt"
line="372"
line="380"
column="29"/>
</issue>
@ -2093,7 +2093,7 @@
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/components/account/AccountActivity.kt"
line="375"
line="383"
column="29"/>
</issue>
@ -2104,7 +2104,7 @@
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/components/account/AccountActivity.kt"
line="381"
line="389"
column="21"/>
</issue>
@ -2115,7 +2115,7 @@
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/components/account/AccountActivity.kt"
line="382"
line="390"
column="21"/>
</issue>
@ -2126,7 +2126,7 @@
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/components/account/AccountActivity.kt"
line="384"
line="392"
column="21"/>
</issue>
@ -2137,7 +2137,7 @@
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/components/account/AccountActivity.kt"
line="394"
line="402"
column="21"/>
</issue>
@ -2159,7 +2159,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/app/pachli/components/preference/AccountPreferencesFragment.kt"
line="318"
line="309"
column="29"/>
</issue>
@ -2170,7 +2170,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/app/pachli/components/preference/AccountPreferencesFragment.kt"
line="324"
line="315"
column="25"/>
</issue>
@ -2324,7 +2324,7 @@
errorLine2=" ~~~~~~~~~">
<location
file="src/main/java/app/pachli/BaseActivity.kt"
line="105"
line="103"
column="13"/>
</issue>
@ -3138,7 +3138,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="725"
line="722"
column="17"/>
</issue>
@ -3149,7 +3149,7 @@
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="728"
line="725"
column="21"/>
</issue>
@ -3158,20 +3158,20 @@
message="Access to `private` method `secondaryDrawerItem` of class `MainActivityKt` requires synthetic accessor"
errorLine1=" secondaryDrawerItem {"
errorLine2=" ~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="730"
column="17"/>
</issue>
<issue
id="SyntheticAccessor"
message="Access to `private` method `setOnClick` of class `MainActivityKt` requires synthetic accessor"
errorLine1=" onClick = {"
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="733"
column="17"/>
</issue>
<issue
id="SyntheticAccessor"
message="Access to `private` method `setOnClick` of class `MainActivityKt` requires synthetic accessor"
errorLine1=" onClick = {"
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="736"
column="21"/>
</issue>
@ -3182,7 +3182,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="741"
line="738"
column="17"/>
</issue>
@ -3193,7 +3193,7 @@
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="744"
line="741"
column="21"/>
</issue>
@ -3204,7 +3204,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="751"
line="748"
column="21"/>
</issue>
@ -3215,7 +3215,7 @@
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="754"
line="751"
column="25"/>
</issue>
@ -3226,7 +3226,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="763"
line="760"
column="17"/>
</issue>
@ -3237,7 +3237,7 @@
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="766"
line="763"
column="21"/>
</issue>
@ -3248,7 +3248,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="779"
line="776"
column="17"/>
</issue>
@ -3259,7 +3259,7 @@
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="783"
line="780"
column="21"/>
</issue>
@ -3270,7 +3270,7 @@
errorLine2=" ~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="908"
line="905"
column="17"/>
</issue>
@ -3281,7 +3281,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="910"
line="907"
column="17"/>
</issue>
@ -3292,7 +3292,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/app/pachli/MainActivity.kt"
line="921"
line="918"
column="17"/>
</issue>
@ -3904,7 +3904,7 @@
errorLine2=" ~~~~~~~~">
<location
file="src/main/res/layout/activity_account.xml"
line="242"
line="243"
column="22"/>
</issue>
@ -3915,7 +3915,7 @@
errorLine2=" ~~~~~~~~">
<location
file="src/main/res/layout/activity_account.xml"
line="278"
line="279"
column="26"/>
</issue>
@ -3926,7 +3926,7 @@
errorLine2=" ~~~~~~~~">
<location
file="src/main/res/layout/activity_account.xml"
line="305"
line="306"
column="26"/>
</issue>
@ -3937,7 +3937,7 @@
errorLine2=" ~~~~~~~~">
<location
file="src/main/res/layout/activity_account.xml"
line="320"
line="321"
column="26"/>
</issue>
@ -3948,7 +3948,7 @@
errorLine2=" ~~~~~~~~">
<location
file="src/main/res/layout/activity_account.xml"
line="347"
line="348"
column="26"/>
</issue>
@ -3959,7 +3959,7 @@
errorLine2=" ~~~~~~~~">
<location
file="src/main/res/layout/activity_account.xml"
line="378"
line="379"
column="26"/>
</issue>
@ -3970,7 +3970,7 @@
errorLine2=" ~~~~~~~~">
<location
file="src/main/res/layout/activity_account.xml"
line="408"
line="409"
column="26"/>
</issue>

View File

@ -28,6 +28,7 @@ import android.net.Uri
import android.os.Build
import android.os.Bundle
import android.provider.MediaStore
import android.text.InputFilter
import android.text.Spanned
import android.text.style.URLSpan
import android.view.KeyEvent
@ -74,6 +75,7 @@ import app.pachli.components.compose.dialog.showAddPollDialog
import app.pachli.components.compose.view.ComposeOptionsListener
import app.pachli.components.compose.view.ComposeScheduleView
import app.pachli.components.instanceinfo.InstanceInfoRepository
import app.pachli.core.common.string.mastodonLength
import app.pachli.core.database.model.AccountEntity
import app.pachli.core.navigation.ComposeActivityIntent
import app.pachli.core.navigation.ComposeActivityIntent.ComposeOptions
@ -1323,6 +1325,7 @@ class ComposeActivity :
* (https://docs.joinmastodon.org/user/posting/#mentions)
* - Hashtags are always treated as their actual length, including the "#"
* (https://docs.joinmastodon.org/user/posting/#hashtags)
* - Emojis are treated as a single character
*
* Content warning text is always treated as its full length, URLs and other entities
* are not treated differently.
@ -1332,9 +1335,8 @@ class ComposeActivity :
* @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)
var length = body.toString().mastodonLength() - 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) {
@ -1347,15 +1349,50 @@ class ComposeActivity :
}
else -> {
// Expected to be negative if the URL length < maxUrlLength
span.url.length - urlLength
span.url.mastodonLength() - urlLength
}
}
}
// Content warning text is treated as is, URLs or mentions there are not special
contentWarning?.let { length += it.length }
contentWarning?.let { length += it.toString().mastodonLength() }
return length
}
/**
* [InputFilter] that uses the "Mastodon" length of a string, where emojis always
* count as a single character.
*
* Unlike [InputFilter.LengthFilter] the source input is not trimmed if it is
* too long, it's just rejected.
*/
class MastodonLengthFilter(private val maxLength: Int) : InputFilter {
override fun filter(
source: CharSequence?,
start: Int,
end: Int,
dest: Spanned?,
dstart: Int,
dend: Int,
): CharSequence? {
val destRemovedLength = dest?.subSequence(dstart, dend).toString().mastodonLength()
val available = maxLength - dest.toString().mastodonLength() + destRemovedLength
val sourceLength = source?.subSequence(start, end).toString().mastodonLength()
// Not enough space to insert the new text
if (sourceLength > available) return REJECT
return ALLOW
}
companion object {
/** Filter result allowing the changes */
val ALLOW = null
/** Filter result preventing the changes */
const val REJECT = ""
}
}
}
}

View File

@ -16,13 +16,13 @@
package app.pachli.components.compose.dialog
import android.text.InputFilter
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.core.widget.doOnTextChanged
import androidx.recyclerview.widget.RecyclerView
import app.pachli.R
import app.pachli.components.compose.ComposeActivity.Companion.MastodonLengthFilter
import app.pachli.databinding.ItemAddPollOptionBinding
import app.pachli.util.BindingHolder
import app.pachli.util.visible
@ -45,7 +45,7 @@ class AddPollOptionsAdapter(
override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): BindingHolder<ItemAddPollOptionBinding> {
val binding = ItemAddPollOptionBinding.inflate(LayoutInflater.from(parent.context), parent, false)
val holder = BindingHolder(binding)
binding.optionEditText.filters = arrayOf(InputFilter.LengthFilter(maxOptionLength))
binding.optionEditText.filters = arrayOf(MastodonLengthFilter(maxOptionLength))
binding.optionEditText.doOnTextChanged { s, _, _, _ ->
val pos = holder.bindingAdapterPosition

View File

@ -1,6 +1,6 @@
package app.pachli
import android.text.Spannable
import app.pachli.core.testing.fakes.FakeSpannable
import app.pachli.util.highlightSpans
import org.junit.Assert
import org.junit.Test
@ -127,55 +127,4 @@ class SpanUtilsTest {
Assert.assertEquals(expectedEndIndex, span.end)
}
}
class FakeSpannable(private val text: String) : Spannable {
val spans = mutableListOf<BoundedSpan>()
override fun setSpan(what: Any?, start: Int, end: Int, flags: Int) {
spans.add(BoundedSpan(what, start, end))
}
override fun <T : Any> getSpans(start: Int, end: Int, type: Class<T>): Array<T> {
return spans.filter { it.start >= start && it.end <= end && type.isInstance(it.span) }
.map { it.span }
.toTypedArray() as Array<T>
}
override fun removeSpan(what: Any?) {
spans.removeIf { span -> span.span == what }
}
override fun toString(): String {
return text
}
override val length: Int
get() = text.length
class BoundedSpan(val span: Any?, val start: Int, val end: Int)
override fun nextSpanTransition(start: Int, limit: Int, type: Class<*>?): Int {
throw NotImplementedError()
}
override fun getSpanEnd(tag: Any?): Int {
throw NotImplementedError()
}
override fun getSpanFlags(tag: Any?): Int {
throw NotImplementedError()
}
override fun get(index: Int): Char {
throw NotImplementedError()
}
override fun subSequence(startIndex: Int, endIndex: Int): CharSequence {
throw NotImplementedError()
}
override fun getSpanStart(tag: Any?): Int {
throw NotImplementedError()
}
}
}

View File

@ -238,6 +238,49 @@ class ComposeActivityTest {
}
}
@Test
fun whenTextContainsEmoji_emojisAreCountedAsOneCharacter() {
val content = "Test 😜"
rule.launch()
rule.getScenario().onActivity {
insertSomeTextInContent(it, content)
assertEquals(6, it.calculateTextLength())
}
}
@Test
fun whenTextContainsConesecutiveEmoji_emojisAreCountedAsSeparateCharacters() {
val content = "Test 😜😜"
rule.launch()
rule.getScenario().onActivity {
insertSomeTextInContent(it, content)
assertEquals(7, it.calculateTextLength())
}
}
@Test
fun whenTextContainsUrlWithEmoji_ellipsizedUrlIsCountedCorrectly() {
val content = "https://🤪.com"
rule.launch()
rule.getScenario().onActivity {
insertSomeTextInContent(it, content)
assertEquals(
InstanceInfoRepository.DEFAULT_CHARACTERS_RESERVED_PER_URL,
it.calculateTextLength(),
)
}
}
@Test
fun whenTextContainsNonEnglishCharacters_lengthIsCountedCorrectly() {
val content = "こんにちは. General Kenobi" // "Hello there. General Kenobi"
rule.launch()
rule.getScenario().onActivity {
insertSomeTextInContent(it, content)
assertEquals(21, it.calculateTextLength())
}
}
@Test
fun whenTextContainsUrl_onlyEllipsizedURLIsCounted() {
val url = "https://www.google.dk/search?biw=1920&bih=990&tbm=isch&sa=1&ei=bmDrWuOoKMv6kwWOkIaoDQ&q=indiana+jones+i+hate+snakes+animated&oq=indiana+jones+i+hate+snakes+animated&gs_l=psy-ab.3...54174.55443.0.55553.9.7.0.0.0.0.255.333.1j0j1.2.0....0...1c.1.64.psy-ab..7.0.0....0.40G-kcDkC6A#imgdii=PSp15hQjN1JqvM:&imgrc=H0hyE2JW5wrpBM:"

View File

@ -0,0 +1,169 @@
/*
* Copyright 2023 Pachli Association
*
* This file is a part of Pachli.
*
* 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.
*
* Pachli 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 Pachli; if not,
* see <http://www.gnu.org/licenses>.
*/
package app.pachli.components.compose
import app.pachli.components.compose.ComposeActivity.Companion.MastodonLengthFilter
import app.pachli.components.compose.ComposeActivity.Companion.MastodonLengthFilter.Companion.ALLOW
import app.pachli.components.compose.ComposeActivity.Companion.MastodonLengthFilter.Companion.REJECT
import app.pachli.core.testing.fakes.FakeSpannable
import com.google.common.truth.Truth.assertThat
import org.junit.Test
import org.junit.runner.RunWith
import org.junit.runners.Parameterized
import org.junit.runners.Parameterized.Parameters
/**
* Parameters to pass to [MastodonLengthFilter.filter] and the expected result.
*/
data class TestData(
val source: CharSequence?,
val start: Int,
val end: Int,
val dest: String,
val dstart: Int,
val dend: Int,
/** The expected result */
val want: String?,
)
@RunWith(Parameterized::class)
class MastodonLengthFilterTest(private val testData: TestData) {
companion object {
@Parameters(name = "{0}")
@JvmStatic
fun data(): Iterable<TestData> {
return listOf(
// Inserting 9 characters to empty string
TestData(
"some text",
0,
9,
"",
0,
0,
ALLOW,
),
// Appending 1 character to length 5 string
TestData(
"6",
0,
1,
"12345",
0,
5,
ALLOW,
),
// Replacing 1 character in middle of length 5 string
TestData(
"x",
0,
1,
"12345",
2,
3,
ALLOW,
),
// Replacing all characters in length 5 string
TestData(
"xxxxx",
0,
4,
"12345",
0,
4,
ALLOW,
),
// Deleting characters with a zero-length replacement
TestData(
"",
0,
0,
"12345",
0,
3,
ALLOW,
),
// Appending a 2 character emoji to a 9 character string
TestData(
"😜",
0,
2,
"123456789",
9,
9,
ALLOW,
),
// Extending a 10 character string by 1
TestData(
"x",
0,
1,
"1234567890",
10,
10,
REJECT,
),
// Extending a 9 character string by 2
TestData(
"xx",
0,
2,
"123456789",
9,
9,
REJECT,
),
// Replacing 2 characters in a 10 character string with 3
TestData(
"xxx",
0,
3,
"1234567890",
5,
7,
REJECT,
),
// Appending 2 x 2 character emojis to a 9 character string
TestData(
"😜😜",
0,
3,
"123456789",
9,
9,
REJECT,
),
)
}
}
@Test
fun filter_matchesExpectations() {
val filter = MastodonLengthFilter(10)
assertThat(
filter.filter(
testData.source,
testData.start,
testData.end,
FakeSpannable(testData.dest),
testData.dstart,
testData.dend,
),
).isEqualTo(testData.want)
}
}

View File

@ -17,7 +17,7 @@
package app.pachli.components.compose
import app.pachli.SpanUtilsTest
import app.pachli.core.testing.fakes.FakeSpannable
import app.pachli.util.highlightSpans
import org.junit.Assert.assertEquals
import org.junit.Test
@ -53,7 +53,7 @@ class StatusLengthTest(
@Test
fun statusLength_matchesExpectations() {
val spannedText = SpanUtilsTest.FakeSpannable(text)
val spannedText = FakeSpannable(text)
highlightSpans(spannedText, 0)
assertEquals(
@ -64,10 +64,10 @@ class StatusLengthTest(
@Test
fun statusLength_withCwText_matchesExpectations() {
val spannedText = SpanUtilsTest.FakeSpannable(text)
val spannedText = FakeSpannable(text)
highlightSpans(spannedText, 0)
val cwText = SpanUtilsTest.FakeSpannable(
val cwText = FakeSpannable(
"a @example@example.org #hashtagmention and http://example.org URL",
)
assertEquals(

View File

@ -1,6 +1,7 @@
package app.pachli.core.common.string
import android.text.Spanned
import java.text.BreakIterator
import java.util.Random
private const val POSSIBLE_CHARS = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"
@ -44,6 +45,20 @@ fun String.isLessThanOrEqual(other: String): Boolean {
return this == other || isLessThan(other)
}
/**
* @return the "Mastodon" length of a string. [String.length] counts emojis as
* multiple characters, but Mastodon treats them as a single character.
*/
fun String.mastodonLength(): Int {
val breakIterator = BreakIterator.getCharacterInstance()
breakIterator.setText(this)
var count = 0
while (breakIterator.next() != BreakIterator.DONE) {
count++
}
return count
}
fun Spanned.trimTrailingWhitespace(): Spanned {
var i = length
do {

View File

@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<issues format="6" by="lint 8.1.2" type="baseline" client="gradle" dependencies="false" name="AGP (8.1.2)" variant="all" version="8.1.2">
<issues format="6" by="lint 8.2.0" type="baseline" client="gradle" dependencies="false" name="AGP (8.2.0)" variant="all" version="8.2.0">
<issue
id="InvalidPackage"
@ -71,6 +71,17 @@
file="$GRADLE_USER_HOME/caches/modules-2/files-2.1/org.robolectric/utils/4.11.1/12828864aac49e8ebcf2de03d3d33919ca8a1b01/utils-4.11.1.jar"/>
</issue>
<issue
id="NewApi"
message="Call requires API level 24 (current min is 23): `java.util.Collection#removeIf`"
errorLine1=" spans.removeIf { span -> span.span == what }"
errorLine2=" ~~~~~~~~">
<location
file="src/main/kotlin/app/pachli/core/testing/fakes/FakeSpannable.kt"
line="36"
column="15"/>
</issue>
<issue
id="NewApi"
message="Call requires API level 24 (current min is 23): `java.util.Map#getOrDefault`"

View File

@ -0,0 +1,71 @@
/*
* Copyright 2023 Pachli Association
*
* This file is a part of Pachli.
*
* 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.
*
* Pachli 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 Pachli; if not,
* see <http://www.gnu.org/licenses>.
*/
package app.pachli.core.testing.fakes
import android.text.Spannable
class FakeSpannable(private val text: String) : Spannable {
val spans = mutableListOf<BoundedSpan>()
override fun setSpan(what: Any?, start: Int, end: Int, flags: Int) {
spans.add(BoundedSpan(what, start, end))
}
override fun <T : Any> getSpans(start: Int, end: Int, type: Class<T>): Array<T> {
return spans.filter { it.start >= start && it.end <= end && type.isInstance(it.span) }
.map { it.span }
.toTypedArray() as Array<T>
}
override fun removeSpan(what: Any?) {
spans.removeIf { span -> span.span == what }
}
override fun toString(): String {
return text
}
override val length: Int
get() = text.length
class BoundedSpan(val span: Any?, val start: Int, val end: Int)
override fun nextSpanTransition(start: Int, limit: Int, type: Class<*>?): Int {
throw NotImplementedError()
}
override fun getSpanEnd(tag: Any?): Int {
throw NotImplementedError()
}
override fun getSpanFlags(tag: Any?): Int {
throw NotImplementedError()
}
override fun get(index: Int): Char {
throw NotImplementedError()
}
override fun subSequence(startIndex: Int, endIndex: Int): CharSequence {
return text.subSequence(startIndex, endIndex)
}
override fun getSpanStart(tag: Any?): Int {
throw NotImplementedError()
}
}