Merge pull request #7348 from hayaksi1/fix/3338-rtl-room-list-preview

Stop a right to left sender name from reordering a room list preview

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,7 @@ class DefaultRoomLatestEventFormatter(
             if (isOutgoing) {
                 sp.getString(CommonStrings.common_you)
             } else {
-                senderDisambiguatedDisplayName
+                senderDisambiguatedDisplayName.bidiIsolate()
             }
         )
     }

libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/PrefixWith.kt

@@ -14,6 +14,12 @@ import androidx.compose.ui.text.buildAnnotatedString
 import androidx.compose.ui.text.font.FontWeight
 import androidx.compose.ui.text.withStyle
 
+private const val FIRST_STRONG_ISOLATE = 0x2068
+private const val POP_DIRECTIONAL_ISOLATE = 0x2069
+
+/** Wraps the string between Unicode isolate characters so its own direction cannot reorder the text around it. */
+internal fun String.bidiIsolate(): String = Char(FIRST_STRONG_ISOLATE) + this + Char(POP_DIRECTIONAL_ISOLATE)
+
 internal fun CharSequence.prefixWith(prefix: String): AnnotatedString {
     return buildAnnotatedString {
         withStyle(SpanStyle(fontWeight = FontWeight.Bold)) {

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,13 @@ 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
+        val spanStart = someoneElseId.value.length + 4
         assertThat(boldSpanStyle!!.start..boldSpanStyle.end).isEqualTo(spanStart..spanStart + 7)
         assertThat(result.toString()).isEqualTo(expectedBody)
     }
@@ -130,7 +130,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 +152,29 @@ 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")
+        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 +183,7 @@ class DefaultRoomLatestEventFormatterTest : RobolectricTest() {
         testMessageContents(
             sentByYou = false,
             senderName = "Alice",
-            expectedPrefix = "Alice",
+            expectedPrefix = "⁨Alice⁩",
         )
     }
 
@@ -233,7 +250,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 +292,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 +961,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 +969,7 @@ 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
+        val spanStart = "Bob".length + 4
         assertThat(boldSpanStyle!!.start..boldSpanStyle.end).isEqualTo(spanStart..spanStart + 4)
     }