diff --git a/changelog.d/4540.bugfix b/changelog.d/4540.bugfix new file mode 100644 index 0000000000..9c962f91f7 --- /dev/null +++ b/changelog.d/4540.bugfix @@ -0,0 +1 @@ +Fix message replies/quotes to respect newlines. \ No newline at end of file diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/room/send/MarkdownParserTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/room/send/MarkdownParserTest.kt index 1ed2f89977..8625e97902 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/room/send/MarkdownParserTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/room/send/MarkdownParserTest.kt @@ -49,6 +49,7 @@ class MarkdownParserTest : InstrumentedTest { * Create the same parser than in the RoomModule */ private val markdownParser = MarkdownParser( + Parser.builder().build(), Parser.builder().build(), HtmlRenderer.builder().softbreak("
").build(), TextPillsUtils( diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/send/SendService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/send/SendService.kt index 5b387c3413..606500c4e7 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/send/SendService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/send/SendService.kt @@ -56,6 +56,15 @@ interface SendService { */ fun sendFormattedTextMessage(text: String, formattedText: String, msgType: String = MessageType.MSGTYPE_TEXT): Cancelable + /** + * Method to quote an events content. + * @param quotedEvent The event to which we will quote it's content. + * @param text the text message to send + * @param autoMarkdown If true, the SDK will generate a formatted HTML message from the body text if markdown syntax is present + * @return a [Cancelable] + */ + fun sendQuotedTextMessage(quotedEvent: TimelineEvent, text: String, autoMarkdown: Boolean): Cancelable + /** * Method to send a media asynchronously. * @param attachment the media to send diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/RoomModule.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/RoomModule.kt index c73318ee0a..64f6bc0b30 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/RoomModule.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/RoomModule.kt @@ -21,6 +21,7 @@ import dagger.Module import dagger.Provides import org.commonmark.Extension import org.commonmark.ext.maths.MathsExtension +import org.commonmark.node.BlockQuote import org.commonmark.parser.Parser import org.commonmark.renderer.html.HtmlRenderer import org.matrix.android.sdk.api.session.file.FileService @@ -100,6 +101,21 @@ import org.matrix.android.sdk.internal.session.room.version.DefaultRoomVersionUp import org.matrix.android.sdk.internal.session.room.version.RoomVersionUpgradeTask import org.matrix.android.sdk.internal.session.space.DefaultSpaceService import retrofit2.Retrofit +import javax.inject.Qualifier + +/** + * Used to inject the simple commonmark Parser + */ +@Qualifier +@Retention(AnnotationRetention.RUNTIME) +internal annotation class SimpleCommonmarkParser + +/** + * Used to inject the advanced commonmark Parser + */ +@Qualifier +@Retention(AnnotationRetention.RUNTIME) +internal annotation class AdvancedCommonmarkParser @Module internal abstract class RoomModule { @@ -123,11 +139,23 @@ internal abstract class RoomModule { } @Provides + @AdvancedCommonmarkParser @JvmStatic - fun providesParser(): Parser { + fun providesAdvancedParser(): Parser { return Parser.builder().extensions(extensions).build() } + @Provides + @SimpleCommonmarkParser + @JvmStatic + fun providesSimpleParser(): Parser { + // The simple parser disables all blocks but quotes. + // Inline parsing(bold, italic, etc) is also enabled and is not easy to disable in commonmark currently. + return Parser.builder() + .enabledBlockTypes(setOf(BlockQuote::class.java)) + .build() + } + @Provides @JvmStatic fun providesHtmlRenderer(): HtmlRenderer { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/DefaultSendService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/DefaultSendService.kt index d3162aef79..fb2fb3950a 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/DefaultSendService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/DefaultSendService.kt @@ -97,6 +97,12 @@ internal class DefaultSendService @AssistedInject constructor( .let { sendEvent(it) } } + override fun sendQuotedTextMessage(quotedEvent: TimelineEvent, text: String, autoMarkdown: Boolean): Cancelable { + return localEchoEventFactory.createQuotedTextEvent(roomId, quotedEvent, text, autoMarkdown) + .also { createLocalEcho(it) } + .let { sendEvent(it) } + } + override fun sendPoll(question: String, options: List): Cancelable { return localEchoEventFactory.createPollEvent(roomId, question, options) .also { createLocalEcho(it) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/LocalEchoEventFactory.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/LocalEchoEventFactory.kt index 85b22628d7..c4caedc407 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/LocalEchoEventFactory.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/LocalEchoEventFactory.kt @@ -198,20 +198,23 @@ internal class LocalEchoEventFactory @Inject constructor( eventReplaced: TimelineEvent, originalEvent: TimelineEvent, newBodyText: String, - newBodyAutoMarkdown: Boolean, + autoMarkdown: Boolean, msgType: String, compatibilityText: String): Event { val permalink = permalinkFactory.createPermalink(roomId, originalEvent.root.eventId ?: "", false) val userLink = originalEvent.root.senderId?.let { permalinkFactory.createPermalink(it, false) } ?: "" val body = bodyForReply(originalEvent.getLastMessageContent(), originalEvent.isReply()) - val replyFormatted = REPLY_PATTERN.format( + // As we always supply formatted body for replies we should force the MarkdownParser to produce html. + val newBodyFormatted = markdownParser.parse(newBodyText, force = true, advanced = autoMarkdown).takeFormatted() + // Body of the original message may not have formatted version, so may also have to convert to html. + val bodyFormatted = body.formattedText ?: markdownParser.parse(body.text, force = true, advanced = autoMarkdown).takeFormatted() + val replyFormatted = buildFormattedReply( permalink, userLink, originalEvent.senderInfo.disambiguatedDisplayName, - // Remove inner mx_reply tags if any - body.takeFormatted().replace(MX_REPLY_REGEX, ""), - createTextContent(newBodyText, newBodyAutoMarkdown).takeFormatted() + bodyFormatted, + newBodyFormatted ) // // > <@alice:example.org> This is the original body @@ -391,13 +394,17 @@ internal class LocalEchoEventFactory @Inject constructor( val userLink = permalinkFactory.createPermalink(userId, false) ?: return null val body = bodyForReply(eventReplied.getLastMessageContent(), eventReplied.isReply()) - val replyFormatted = REPLY_PATTERN.format( + + // As we always supply formatted body for replies we should force the MarkdownParser to produce html. + val replyTextFormatted = markdownParser.parse(replyText, force = true, advanced = autoMarkdown).takeFormatted() + // Body of the original message may not have formatted version, so may also have to convert to html. + val bodyFormatted = body.formattedText ?: markdownParser.parse(body.text, force = true, advanced = autoMarkdown).takeFormatted() + val replyFormatted = buildFormattedReply( permalink, userLink, userId, - // Remove inner mx_reply tags if any - body.takeFormatted().replace(MX_REPLY_REGEX, ""), - createTextContent(replyText, autoMarkdown).takeFormatted() + bodyFormatted, + replyTextFormatted ) // // > <@alice:example.org> This is the original body @@ -415,6 +422,16 @@ internal class LocalEchoEventFactory @Inject constructor( return createMessageEvent(roomId, content) } + private fun buildFormattedReply(permalink: String, userLink: String, userId: String, bodyFormatted: String, newBodyFormatted: String): String { + return REPLY_PATTERN.format( + permalink, + userLink, + userId, + // Remove inner mx_reply tags if any + bodyFormatted.replace(MX_REPLY_REGEX, ""), + newBodyFormatted + ) + } private fun buildReplyFallback(body: TextContent, originalSenderId: String?, newBodyText: String): String { return buildString { append("> <") @@ -498,6 +515,38 @@ internal class LocalEchoEventFactory @Inject constructor( localEchoRepository.createLocalEcho(event) } + fun createQuotedTextEvent( + roomId: String, + quotedEvent: TimelineEvent, + text: String, + autoMarkdown: Boolean, + ): Event { + val messageContent = quotedEvent.getLastMessageContent() + val textMsg = messageContent?.body + val quoteText = legacyRiotQuoteText(textMsg, text) + return createFormattedTextEvent(roomId, markdownParser.parse(quoteText, force = true, advanced = autoMarkdown), MessageType.MSGTYPE_TEXT) + } + + private fun legacyRiotQuoteText(quotedText: String?, myText: String): String { + val messageParagraphs = quotedText?.split("\n\n".toRegex())?.dropLastWhile { it.isEmpty() }?.toTypedArray() + return buildString { + if (messageParagraphs != null) { + for (i in messageParagraphs.indices) { + if (messageParagraphs[i].isNotBlank()) { + append("> ") + append(messageParagraphs[i]) + } + + if (i != messageParagraphs.lastIndex) { + append("\n\n") + } + } + } + append("\n\n") + append(myText) + } + } + companion object { // //
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/MarkdownParser.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/MarkdownParser.kt index 1ac95154f8..ef7945cf8c 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/MarkdownParser.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/MarkdownParser.kt @@ -18,6 +18,8 @@ package org.matrix.android.sdk.internal.session.room.send import org.commonmark.parser.Parser import org.commonmark.renderer.html.HtmlRenderer +import org.matrix.android.sdk.internal.session.room.AdvancedCommonmarkParser +import org.matrix.android.sdk.internal.session.room.SimpleCommonmarkParser import org.matrix.android.sdk.internal.session.room.send.pills.TextPillsUtils import javax.inject.Inject @@ -27,22 +29,30 @@ import javax.inject.Inject * If any change is required, please add a test covering the problem and make sure all the tests are still passing. */ internal class MarkdownParser @Inject constructor( - private val parser: Parser, + @AdvancedCommonmarkParser private val advancedParser: Parser, + @SimpleCommonmarkParser private val simpleParser: Parser, private val htmlRenderer: HtmlRenderer, private val textPillsUtils: TextPillsUtils ) { private val mdSpecialChars = "[`_\\-*>.\\[\\]#~$]".toRegex() - fun parse(text: CharSequence): TextContent { + /** + * Parses some input text and produces html. + * @param text An input CharSequence to be parsed. + * @param force Skips the check for detecting if the input contains markdown and always converts to html. + * @param advanced Whether to use the full markdown support or the simple version. + * @return TextContent containing the plain text and the formatted html if generated. + */ + fun parse(text: CharSequence, force: Boolean = false, advanced: Boolean = true): TextContent { val source = textPillsUtils.processSpecialSpansToMarkdown(text) ?: text.toString() // If no special char are detected, just return plain text - if (source.contains(mdSpecialChars).not()) { + if (!force && source.contains(mdSpecialChars).not()) { return TextContent(source) } - val document = parser.parse(source) + val document = if (advanced) advancedParser.parse(source) else simpleParser.parse(source) val htmlText = htmlRenderer.render(document) // Cleanup extra paragraph diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt index 9a18c65a51..a63a06928a 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt @@ -39,8 +39,6 @@ import im.vector.app.features.settings.VectorPreferences import im.vector.app.features.voice.VoicePlayerHelper import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch -import org.commonmark.parser.Parser -import org.commonmark.renderer.html.HtmlRenderer import org.matrix.android.sdk.api.query.QueryStringValue import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.content.ContentAttachmentData @@ -408,23 +406,7 @@ class MessageComposerViewModel @AssistedInject constructor( popDraft() } is SendMode.Quote -> { - val messageContent = state.sendMode.timelineEvent.getLastMessageContent() - val textMsg = messageContent?.body - - val finalText = legacyRiotQuoteText(textMsg, action.text.toString()) - - // TODO check for pills? - - // TODO Refactor this, just temporary for quotes - val parser = Parser.builder().build() - val document = parser.parse(finalText) - val renderer = HtmlRenderer.builder().build() - val htmlText = renderer.render(document) - if (finalText == htmlText) { - room.sendTextMessage(finalText) - } else { - room.sendFormattedTextMessage(finalText, htmlText) - } + room.sendQuotedTextMessage(state.sendMode.timelineEvent, action.text.toString(), action.autoMarkdown) _viewEvents.post(MessageComposerViewEvents.MessageSent) popDraft() }