Merge pull request #1802 from vector-im/feature/fix_reply_tag

Feature/fix reply tag

CHANGES.md

@@ -18,6 +18,7 @@ Bugfix 🐛:
  - Fix 404 on EMS (#1761)
  - Fix Infinite loop at startup when migrating account from Riot (#1699)
  - Fix Element crashes in loop after initial sync (#1709)
+ - Remove inner mx-reply tags before replying
  - Fix timeline items not loading when there are only filtered events
  - Fix "Voice & Video" grayed out in Settings (#1733)
  - Fix Allow VOIP call in all rooms with 2 participants (even if not DM)

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/events/model/Event.kt

@@ -21,9 +21,11 @@ import com.squareup.moshi.JsonClass
 import im.vector.matrix.android.api.session.crypto.MXCryptoError
 import im.vector.matrix.android.api.session.room.model.message.MessageContent
 import im.vector.matrix.android.api.session.room.model.message.MessageType
+import im.vector.matrix.android.api.session.room.model.relation.RelationDefaultContent
 import im.vector.matrix.android.api.session.room.send.SendState
 import im.vector.matrix.android.api.util.JsonDict
 import im.vector.matrix.android.internal.crypto.algorithms.olm.OlmDecryptionResult
+import im.vector.matrix.android.internal.crypto.model.event.EncryptedEventContent
 import im.vector.matrix.android.internal.di.MoshiProvider
 import org.json.JSONObject
 import timber.log.Timber
@@ -240,6 +242,18 @@ fun Event.isFileMessage(): Boolean {
     return getClearType() == EventType.MESSAGE
             && when (getClearContent()?.toModel<MessageContent>()?.msgType) {
         MessageType.MSGTYPE_FILE -> true
-        else                      -> false
+        else                     -> false
     }
 }
+
+fun Event.getRelationContent(): RelationDefaultContent? {
+    return if (isEncrypted()) {
+        content.toModel<EncryptedEventContent>()?.relatesTo
+    } else {
+        content.toModel<MessageContent>()?.relatesTo
+    }
+}
+
+fun Event.isReply(): Boolean {
+    return getRelationContent()?.inReplyTo?.eventId != null
+}

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/model/message/MessageContent.kt

@@ -25,7 +25,3 @@ interface MessageContent {
     val relatesTo: RelationDefaultContent?
     val newContent: Content?
 }
-
-fun MessageContent?.isReply(): Boolean {
-    return this?.relatesTo?.inReplyTo?.eventId != null
-}

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/timeline/TimelineEvent.kt

@@ -20,15 +20,16 @@ import im.vector.matrix.android.BuildConfig
 import im.vector.matrix.android.api.session.events.model.Event
 import im.vector.matrix.android.api.session.events.model.EventType
 import im.vector.matrix.android.api.session.events.model.RelationType
+import im.vector.matrix.android.api.session.events.model.getRelationContent
+import im.vector.matrix.android.api.session.events.model.isReply
 import im.vector.matrix.android.api.session.events.model.toModel
 import im.vector.matrix.android.api.session.room.model.EventAnnotationsSummary
 import im.vector.matrix.android.api.session.room.model.ReadReceipt
 import im.vector.matrix.android.api.session.room.model.message.MessageContent
 import im.vector.matrix.android.api.session.room.model.message.MessageStickerContent
-import im.vector.matrix.android.api.session.room.model.message.isReply
+import im.vector.matrix.android.api.session.room.model.relation.RelationDefaultContent
 import im.vector.matrix.android.api.session.room.sender.SenderInfo
 import im.vector.matrix.android.api.util.ContentUtils.extractUsefulTextFromReply
-import im.vector.matrix.android.internal.crypto.model.event.EncryptedEventContent
 
 /**
  * This data class is a wrapper around an Event. It allows to get useful data in the context of a timeline.
@@ -88,11 +89,18 @@ data class TimelineEvent(
  */
 fun TimelineEvent.hasBeenEdited() = annotations?.editSummary != null
 
+/**
+ * Get the relation content if any
+ */
+fun TimelineEvent.getRelationContent(): RelationDefaultContent? {
+    return root.getRelationContent()
+}
+
 /**
  * Get the eventId which was edited by this event if any
  */
 fun TimelineEvent.getEditedEventId(): String? {
-    return root.getClearContent().toModel<MessageContent>()?.relatesTo?.takeIf { it.type == RelationType.REPLACE }?.eventId
+    return getRelationContent()?.takeIf { it.type == RelationType.REPLACE }?.eventId
 }
 
 /**
@@ -121,11 +129,16 @@ fun TimelineEvent.getLastMessageBody(): String? {
     return null
 }
 
+/**
+ * Returns true if it's a reply
+ */
+fun TimelineEvent.isReply(): Boolean {
+    return root.isReply()
+}
+
 fun TimelineEvent.getTextEditableContent(): String? {
-    val originalContent = root.getClearContent().toModel<MessageContent>() ?: return null
-    val isReply = originalContent.isReply() || root.content.toModel<EncryptedEventContent>()?.relatesTo?.inReplyTo?.eventId != null
     val lastContent = getLastMessageContent()
-    return if (isReply) {
+    return if (isReply()) {
         return extractUsefulTextFromReply(lastContent?.body ?: "")
     } else {
         lastContent?.body ?: ""

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt

@@ -30,7 +30,6 @@ import im.vector.matrix.android.api.session.events.model.LocalEcho
 import im.vector.matrix.android.api.session.events.model.RelationType
 import im.vector.matrix.android.api.session.events.model.UnsignedData
 import im.vector.matrix.android.api.session.events.model.toContent
-import im.vector.matrix.android.api.session.events.model.toModel
 import im.vector.matrix.android.api.session.room.model.message.AudioInfo
 import im.vector.matrix.android.api.session.room.model.message.FileInfo
 import im.vector.matrix.android.api.session.room.model.message.ImageInfo
@@ -50,13 +49,13 @@ import im.vector.matrix.android.api.session.room.model.message.OPTION_TYPE_POLL
 import im.vector.matrix.android.api.session.room.model.message.OptionItem
 import im.vector.matrix.android.api.session.room.model.message.ThumbnailInfo
 import im.vector.matrix.android.api.session.room.model.message.VideoInfo
-import im.vector.matrix.android.api.session.room.model.message.isReply
 import im.vector.matrix.android.api.session.room.model.relation.ReactionContent
 import im.vector.matrix.android.api.session.room.model.relation.ReactionInfo
 import im.vector.matrix.android.api.session.room.model.relation.RelationDefaultContent
 import im.vector.matrix.android.api.session.room.model.relation.ReplyToContent
 import im.vector.matrix.android.api.session.room.timeline.TimelineEvent
 import im.vector.matrix.android.api.session.room.timeline.getLastMessageContent
+import im.vector.matrix.android.api.session.room.timeline.isReply
 import im.vector.matrix.android.internal.di.UserId
 import im.vector.matrix.android.internal.session.content.ThumbnailExtractor
 import im.vector.matrix.android.internal.session.room.send.pills.TextPillsUtils
@@ -173,12 +172,13 @@ internal class LocalEchoEventFactory @Inject constructor(
         val userLink = originalEvent.root.senderId?.let { PermalinkFactory.createPermalink(it) }
                 ?: ""
 
-        val body = bodyForReply(originalEvent.getLastMessageContent(), originalEvent.root.getClearContent().toModel())
+        val body = bodyForReply(originalEvent.getLastMessageContent(), originalEvent.isReply())
         val replyFormatted = REPLY_PATTERN.format(
                 permalink,
                 userLink,
                 originalEvent.senderInfo.disambiguatedDisplayName,
-                body.takeFormatted(),
+                // Remove inner mx_reply tags if any
+                body.takeFormatted().replace(MX_REPLY_REGEX, ""),
                 createTextContent(newBodyText, newBodyAutoMarkdown).takeFormatted()
         )
         //
@@ -367,12 +367,13 @@ internal class LocalEchoEventFactory @Inject constructor(
         val userId = eventReplied.root.senderId ?: return null
         val userLink = PermalinkFactory.createPermalink(userId) ?: return null
 
-        val body = bodyForReply(eventReplied.getLastMessageContent(), eventReplied.root.getClearContent().toModel())
+        val body = bodyForReply(eventReplied.getLastMessageContent(), eventReplied.isReply())
         val replyFormatted = REPLY_PATTERN.format(
                 permalink,
                 userLink,
                 userId,
-                body.takeFormatted(),
+                // Remove inner mx_reply tags if any
+                body.takeFormatted().replace(MX_REPLY_REGEX, ""),
                 createTextContent(replyText, autoMarkdown).takeFormatted()
         )
         //
@@ -412,10 +413,10 @@ internal class LocalEchoEventFactory @Inject constructor(
 
     /**
      * Returns a TextContent used for the fallback event representation in a reply message.
-     * We also pass the original content, because in case of an edit of a reply the last content is not
+     * In case of an edit of a reply the last content is not
      * himself a reply, but it will contain the fallbacks, so we have to trim them.
      */
-    private fun bodyForReply(content: MessageContent?, originalContent: MessageContent?): TextContent {
+    private fun bodyForReply(content: MessageContent?, isReply: Boolean): TextContent {
         when (content?.msgType) {
             MessageType.MSGTYPE_EMOTE,
             MessageType.MSGTYPE_TEXT,
@@ -424,7 +425,6 @@ internal class LocalEchoEventFactory @Inject constructor(
                 if (content is MessageContentWithFormattedBody) {
                     formattedText = content.matrixFormattedBody
                 }
-                val isReply = content.isReply() || originalContent.isReply()
                 return if (isReply) {
                     TextContent(content.body, formattedText).removeInReplyFallbacks()
                 } else {
@@ -485,5 +485,8 @@ internal class LocalEchoEventFactory @Inject constructor(
         // </mx-reply>
         // No whitespace because currently breaks temporary formatted text to Span
         const val REPLY_PATTERN = """<mx-reply><blockquote><a href="%s">In reply to</a> <a href="%s">%s</a><br />%s</blockquote></mx-reply>%s"""
+
+        // This is used to replace inner mx-reply tags
+        val MX_REPLY_REGEX = "<mx-reply>.*</mx-reply>".toRegex()
     }
 }

vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/edithistory/ViewEditHistoryViewModel.kt

@@ -30,9 +30,7 @@ import im.vector.matrix.android.api.MatrixCallback
 import im.vector.matrix.android.api.session.Session
 import im.vector.matrix.android.api.session.crypto.MXCryptoError
 import im.vector.matrix.android.api.session.events.model.Event
-import im.vector.matrix.android.api.session.events.model.toModel
+import im.vector.matrix.android.api.session.events.model.isReply
-import im.vector.matrix.android.api.session.room.model.message.MessageContent
-import im.vector.matrix.android.api.session.room.model.message.isReply
 import im.vector.matrix.android.internal.crypto.algorithms.olm.OlmDecryptionResult
 import im.vector.riotx.core.date.VectorDateFormatter
 import im.vector.riotx.core.platform.EmptyAction
@@ -113,7 +111,7 @@ class ViewEditHistoryViewModel @AssistedInject constructor(@Assisted
                         }
 
                         if (event.eventId == it.eventId) {
-                            originalIsReply = it.getClearContent().toModel<MessageContent>().isReply()
+                            originalIsReply = it.isReply()
                         }
                     }
                 }

vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MessageItemFactory.kt

@@ -354,16 +354,22 @@ class MessageItemFactory @Inject constructor(
             when (codeVisitor.codeKind) {
                 CodeVisitor.Kind.BLOCK  -> {
                     val codeFormattedBlock = htmlRenderer.get().render(localFormattedBody)
-                    buildCodeBlockItem(codeFormattedBlock, informationData, highlight, callback, attributes)
+                    if (codeFormattedBlock == null) {
+                        buildFormattedTextItem(messageContent, informationData, highlight, callback, attributes)
+                    } else {
+                        buildCodeBlockItem(codeFormattedBlock, informationData, highlight, callback, attributes)
+                    }
                 }
                 CodeVisitor.Kind.INLINE -> {
                     val codeFormatted = htmlRenderer.get().render(localFormattedBody)
-                    buildMessageTextItem(codeFormatted, false, informationData, highlight, callback, attributes)
+                    if (codeFormatted == null) {
+                        buildFormattedTextItem(messageContent, informationData, highlight, callback, attributes)
+                    } else {
+                        buildMessageTextItem(codeFormatted, false, informationData, highlight, callback, attributes)
+                    }
                 }
                 CodeVisitor.Kind.NONE   -> {
-                    val compressed = htmlCompressor.compress(messageContent.formattedBody!!)
+                    buildFormattedTextItem(messageContent, informationData, highlight, callback, attributes)
-                    val formattedBody = htmlRenderer.get().render(compressed)
-                    buildMessageTextItem(formattedBody, true, informationData, highlight, callback, attributes)
                 }
             }
         } else {
@@ -371,6 +377,16 @@ class MessageItemFactory @Inject constructor(
         }
     }
 
+    private fun buildFormattedTextItem(messageContent: MessageTextContent,
+                                       informationData: MessageInformationData,
+                                       highlight: Boolean,
+                                       callback: TimelineEventController.Callback?,
+                                       attributes: AbsMessageItem.Attributes): MessageTextItem? {
+        val compressed = htmlCompressor.compress(messageContent.formattedBody!!)
+        val formattedBody = htmlRenderer.get().render(compressed)
+        return buildMessageTextItem(formattedBody, true, informationData, highlight, callback, attributes)
+    }
+
     private fun buildMessageTextItem(body: CharSequence,
                                      isFormatted: Boolean,
                                      informationData: MessageInformationData,

vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/format/DisplayableEventFormatter.kt

@@ -19,11 +19,11 @@ package im.vector.riotx.features.home.room.detail.timeline.format
 import im.vector.matrix.android.api.session.events.model.EventType
 import im.vector.matrix.android.api.session.events.model.toModel
 import im.vector.matrix.android.api.session.room.model.message.MessageType
-import im.vector.matrix.android.api.session.room.model.message.isReply
 import im.vector.matrix.android.api.session.room.model.relation.ReactionContent
 import im.vector.matrix.android.api.session.room.timeline.TimelineEvent
 import im.vector.matrix.android.api.session.room.timeline.getLastMessageContent
 import im.vector.matrix.android.api.session.room.timeline.getTextEditableContent
+import im.vector.matrix.android.api.session.room.timeline.isReply
 import im.vector.riotx.EmojiCompatWrapper
 import im.vector.riotx.R
 import im.vector.riotx.core.resources.ColorProvider
@@ -79,13 +79,13 @@ class DisplayableEventFormatter @Inject constructor(
                             return simpleFormat(senderName, stringProvider.getString(R.string.sent_a_file), appendAuthor)
                         }
                         MessageType.MSGTYPE_TEXT                 -> {
-                            if (messageContent.isReply()) {
+                            return if (timelineEvent.isReply()) {
                                 // Skip reply prefix, and show important
                                 // TODO add a reply image span ?
-                                return simpleFormat(senderName, timelineEvent.getTextEditableContent()
+                                simpleFormat(senderName, timelineEvent.getTextEditableContent()
                                         ?: messageContent.body, appendAuthor)
                             } else {
-                                return simpleFormat(senderName, messageContent.body, appendAuthor)
+                                simpleFormat(senderName, messageContent.body, appendAuthor)
                             }
                         }
                         else                                     -> {

vector/src/main/java/im/vector/riotx/features/html/EventHtmlRenderer.kt

@@ -25,6 +25,7 @@ import io.noties.markwon.Markwon
 import io.noties.markwon.html.HtmlPlugin
 import io.noties.markwon.html.TagHandlerNoOp
 import org.commonmark.node.Node
+import timber.log.Timber
 import javax.inject.Inject
 import javax.inject.Singleton
 
@@ -41,11 +42,21 @@ class EventHtmlRenderer @Inject constructor(context: Context,
     }
 
     fun render(text: String): CharSequence {
-        return markwon.toMarkdown(text)
+        return try {
+            markwon.toMarkdown(text)
+        } catch (failure: Throwable) {
+            Timber.v("Fail to render $text to html")
+            text
+        }
     }
 
-    fun render(node: Node): CharSequence {
+    fun render(node: Node): CharSequence? {
-        return markwon.render(node)
+        return try {
+            markwon.render(node)
+        } catch (failure: Throwable) {
+            Timber.v("Fail to render $node to html")
+            return null
+        }
     }
 }