From 75ab17af5733eb0b026ef414f2f4a1e9fe2f1317 Mon Sep 17 00:00:00 2001 From: hayaksi1 <193020925+hayaksi1@users.noreply.github.com> Date: Sun, 9 Aug 2026 03:51:38 +0300 Subject: [PATCH] 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 --- .../impl/DefaultRoomLatestEventFormatter.kt | 6 ++- .../eventformatter/impl/PrefixWith.kt | 10 +++++ .../DefaultRoomLatestEventFormatterTest.kt | 40 ++++++++++++++----- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatter.kt b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatter.kt index 1c5cd72977..6bdc8aa841 100644 --- a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatter.kt +++ b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatter.kt @@ -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() } ) } diff --git a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/PrefixWith.kt b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/PrefixWith.kt index 1a04893e07..41150a3390 100644 --- a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/PrefixWith.kt +++ b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/PrefixWith.kt @@ -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)) { diff --git a/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatterTest.kt b/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatterTest.kt index 791a363f77..abc3cf280d 100644 --- a/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatterTest.kt +++ b/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatterTest.kt @@ -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) }