mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-11-02 12:07:28 +00:00
Stop a right to left sender name from reordering a room list preview
A room list preview is one paragraph, and its direction is resolved from the first strong character. A sender whose display name is Arabic or Hebrew therefore flipped the whole line: the message body moved to the wrong side of the name and the separating colon jumped to the front. The sender's display name is now wrapped in Unicode isolate characters before it is used as a prefix, so its own direction cannot escape it. Both places that interpolate the name are covered: the prefix path and the emote path, which bypasses it. prefixWith itself is deliberately left alone, since the pinned messages banner shares it and only ever passes localised nouns. The "You" prefix is localised rather than user-provided and is not isolated. Relates to https://github.com/element-hq/element-x-android/issues/3338
This commit is contained in:
+4
-2
@@ -135,7 +135,7 @@ class DefaultRoomLatestEventFormatter(
|
||||
val message = when (val messageType: MessageType = type) {
|
||||
// Doesn't need a prefix
|
||||
is EmoteMessageType -> {
|
||||
return "* $senderDisambiguatedDisplayName ${messageType.body}"
|
||||
return "* ${senderDisambiguatedDisplayName.bidiIsolate()} ${messageType.body}"
|
||||
}
|
||||
is TextMessageType -> {
|
||||
messageType.toPlainText(permalinkParser)
|
||||
@@ -189,7 +189,9 @@ class DefaultRoomLatestEventFormatter(
|
||||
if (isOutgoing) {
|
||||
sp.getString(CommonStrings.common_you)
|
||||
} else {
|
||||
senderDisambiguatedDisplayName
|
||||
// The display name is user-provided and may be right-to-left, which would otherwise
|
||||
// reorder the whole preview line.
|
||||
senderDisambiguatedDisplayName.bidiIsolate()
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
+10
@@ -14,6 +14,16 @@ import androidx.compose.ui.text.buildAnnotatedString
|
||||
import androidx.compose.ui.text.font.FontWeight
|
||||
import androidx.compose.ui.text.withStyle
|
||||
|
||||
/**
|
||||
* Wraps the string between Unicode isolate characters (FSI … PDI) so its own direction cannot
|
||||
* reorder the text around it.
|
||||
*
|
||||
* A room list preview is rendered as a single paragraph, so a right-to-left display name otherwise
|
||||
* flips the whole line — the message body ends up on the wrong side of the name and the trailing
|
||||
* colon jumps to the front. See https://github.com/element-hq/element-x-android/issues/3338
|
||||
*/
|
||||
internal fun String.bidiIsolate(): String = "$this"
|
||||
|
||||
internal fun CharSequence.prefixWith(prefix: String): AnnotatedString {
|
||||
return buildAnnotatedString {
|
||||
withStyle(SpanStyle(fontWeight = FontWeight.Bold)) {
|
||||
|
||||
+30
-10
@@ -91,7 +91,7 @@ class DefaultRoomLatestEventFormatterTest : RobolectricTest() {
|
||||
assertThat(result).isEqualTo(expected)
|
||||
} else {
|
||||
assertThat(result).isInstanceOf(AnnotatedString::class.java)
|
||||
assertThat(result.toString()).isEqualTo("$senderName: $expected")
|
||||
assertThat(result.toString()).isEqualTo("$senderName: $expected")
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -103,13 +103,14 @@ class DefaultRoomLatestEventFormatterTest : RobolectricTest() {
|
||||
val info = ImageInfo(null, null, null, null, null, null, null)
|
||||
val message = createLatestEvent(false, null, aStickerContent(body, info, aMediaSource(url = "url")))
|
||||
val result = formatter.format(message, false)
|
||||
val expectedBody = someoneElseId.value + ": Sticker: a sticker body"
|
||||
val expectedBody = "" + someoneElseId.value + ": Sticker: a sticker body"
|
||||
// Check we have formatting
|
||||
assertThat(result is AnnotatedString).isTrue()
|
||||
// And there is a bold span for the 'Sticker' part
|
||||
val boldSpanStyle = (result as AnnotatedString).spanStyles.lastOrNull { it.item.fontWeight == FontWeight.Bold }
|
||||
assertThat(boldSpanStyle).isNotNull()
|
||||
val spanStart = someoneElseId.value.length + 2
|
||||
// +2 for the isolate characters around the sender name, +2 for the ": " separator
|
||||
val spanStart = someoneElseId.value.length + 4
|
||||
assertThat(boldSpanStyle!!.start..boldSpanStyle.end).isEqualTo(spanStart..spanStart + 7)
|
||||
assertThat(result.toString()).isEqualTo(expectedBody)
|
||||
}
|
||||
@@ -130,7 +131,7 @@ class DefaultRoomLatestEventFormatterTest : RobolectricTest() {
|
||||
assertThat(result).isEqualTo(expected)
|
||||
} else {
|
||||
assertThat(result).isInstanceOf(AnnotatedString::class.java)
|
||||
assertThat(result.toString()).isEqualTo("$senderName: $expected")
|
||||
assertThat(result.toString()).isEqualTo("$senderName: $expected")
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -152,12 +153,30 @@ class DefaultRoomLatestEventFormatterTest : RobolectricTest() {
|
||||
assertWithMessage("$type was not properly handled").that(result).isEqualTo(expected)
|
||||
} else {
|
||||
assertWithMessage("$type does not create an AnnotatedString").that(result).isInstanceOf(AnnotatedString::class.java)
|
||||
assertWithMessage("$type was not properly handled").that(result.toString()).isEqualTo("$senderName: $expected")
|
||||
assertWithMessage("$type was not properly handled").that(result.toString()).isEqualTo("$senderName: $expected")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
@Config(qualifiers = "en")
|
||||
fun `a right to left sender name is isolated so it cannot reorder the preview`() {
|
||||
val senderName = "مرحبا"
|
||||
val body = "Hello"
|
||||
val message = createLatestEvent(
|
||||
sentByYou = false,
|
||||
senderDisplayName = senderName,
|
||||
content = MessageContent(body, null, false, null, TextMessageType(body, null)),
|
||||
)
|
||||
val result = formatter.format(message, false)
|
||||
assertThat(result.toString()).isEqualTo("$senderName: $body")
|
||||
// The bold span still covers exactly the name and its isolates, not the body.
|
||||
val boldSpanStyle = (result as AnnotatedString).spanStyles.first { it.item.fontWeight == FontWeight.Bold }
|
||||
assertThat(boldSpanStyle.start).isEqualTo(0)
|
||||
assertThat(boldSpanStyle.end).isEqualTo(senderName.length + 2)
|
||||
}
|
||||
|
||||
// region Message contents
|
||||
|
||||
@Test
|
||||
@@ -166,7 +185,7 @@ class DefaultRoomLatestEventFormatterTest : RobolectricTest() {
|
||||
testMessageContents(
|
||||
sentByYou = false,
|
||||
senderName = "Alice",
|
||||
expectedPrefix = "Alice",
|
||||
expectedPrefix = "Alice",
|
||||
)
|
||||
}
|
||||
|
||||
@@ -233,7 +252,7 @@ class DefaultRoomLatestEventFormatterTest : RobolectricTest() {
|
||||
is StickerMessageType -> "Sticker: Shared body"
|
||||
is FileMessageType -> "File: Shared body"
|
||||
is LocationMessageType -> "Shared location"
|
||||
is EmoteMessageType -> "* $senderName ${type.body}"
|
||||
is EmoteMessageType -> "* $senderName ${type.body}"
|
||||
is TextMessageType,
|
||||
is NoticeMessageType,
|
||||
is OtherMessageType -> body
|
||||
@@ -275,7 +294,7 @@ class DefaultRoomLatestEventFormatterTest : RobolectricTest() {
|
||||
is TextMessageType,
|
||||
is NoticeMessageType,
|
||||
is OtherMessageType -> "$expectedPrefix: $body"
|
||||
is EmoteMessageType -> "* $senderName ${type.body}"
|
||||
is EmoteMessageType -> "* $senderName ${type.body}"
|
||||
}
|
||||
val shouldCreateAnnotatedString = when (type) {
|
||||
is VideoMessageType -> true
|
||||
@@ -944,7 +963,7 @@ class DefaultRoomLatestEventFormatterTest : RobolectricTest() {
|
||||
assertThat(formatter.format(mineContentEvent, false).toString()).isEqualTo("You: Poll: Do you like polls?")
|
||||
|
||||
val contentEvent = createLatestEvent(sentByYou = false, senderDisplayName = "Bob", content = pollContent)
|
||||
assertThat(formatter.format(contentEvent, false).toString()).isEqualTo("Bob: Poll: Do you like polls?")
|
||||
assertThat(formatter.format(contentEvent, false).toString()).isEqualTo("Bob: Poll: Do you like polls?")
|
||||
|
||||
val result = formatter.format(contentEvent, false)
|
||||
// Check we have formatting
|
||||
@@ -952,7 +971,8 @@ class DefaultRoomLatestEventFormatterTest : RobolectricTest() {
|
||||
// And there is a bold span for the 'Poll' part
|
||||
val boldSpanStyle = (result as AnnotatedString).spanStyles.lastOrNull { it.item.fontWeight == FontWeight.Bold }
|
||||
assertThat(boldSpanStyle).isNotNull()
|
||||
val spanStart = "Bob".length + 2
|
||||
// +2 for the isolate characters around the sender name, +2 for the ": " separator
|
||||
val spanStart = "Bob".length + 4
|
||||
assertThat(boldSpanStyle!!.start..boldSpanStyle.end).isEqualTo(spanStart..spanStart + 4)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user