From b40334f7db4a4bada540aca28c2604c3fe78fa84 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Tue, 3 Nov 2020 11:53:50 +0300 Subject: [PATCH 1/3] Markdown body should be pure text, see #739, #1506. --- CHANGES.md | 1 + .../room/send/LocalEchoEventFactory.kt | 3 +-- .../session/room/send/MarkdownParser.kt | 21 ++++++++++++------- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index bb52452f8d..1c348de001 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -22,6 +22,7 @@ Improvements 🙌: Bugfix 🐛: - Messages encrypted with no way to decrypt after SDK update from 0.18 to 1.0.0 (#2252) - Search Result | scroll jumps after pagination (#2238) + - Badly formatted mentions in body (#1506) Translations 🗣: - 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 da3e0429b0..c01923055b 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 @@ -90,8 +90,7 @@ internal class LocalEchoEventFactory @Inject constructor( private fun createTextContent(text: CharSequence, autoMarkdown: Boolean): TextContent { if (autoMarkdown) { - val source = textPillsUtils.processSpecialSpansToMarkdown(text) ?: text.toString() - return markdownParser.parse(source) + return markdownParser.parse(text) } else { // Try to detect pills textPillsUtils.processSpecialSpansToHtml(text)?.let { 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 030c9deb6a..c99d482300 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,7 @@ 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.send.pills.TextPillsUtils import javax.inject.Inject /** @@ -27,18 +28,21 @@ import javax.inject.Inject */ internal class MarkdownParser @Inject constructor( private val parser: Parser, - private val htmlRenderer: HtmlRenderer + private val htmlRenderer: HtmlRenderer, + private val textPillsUtils: TextPillsUtils ) { private val mdSpecialChars = "[`_\\-*>.\\[\\]#~]".toRegex() - fun parse(text: String): TextContent { + fun parse(text: CharSequence): TextContent { + val source = textPillsUtils.processSpecialSpansToMarkdown(text) ?: text.toString() + // If no special char are detected, just return plain text - if (text.contains(mdSpecialChars).not()) { - return TextContent(text) + if (source.contains(mdSpecialChars).not()) { + return TextContent(source) } - val document = parser.parse(text) + val document = parser.parse(source) val htmlText = htmlRenderer.render(document) // Cleanup extra paragraph @@ -48,13 +52,14 @@ internal class MarkdownParser @Inject constructor( htmlText } - return if (isFormattedTextPertinent(text, cleanHtmlText)) { + return if (isFormattedTextPertinent(source, cleanHtmlText)) { // According to https://matrix.org/docs/spec/client_server/latest#m-room-message-msgtypes: // The plain text version of the HTML should be provided in the body. // But it caused too many problems so it has been removed in #2002 - TextContent(text, cleanHtmlText.postTreatment()) + // See #739 + TextContent(text.toString(), cleanHtmlText.postTreatment()) } else { - TextContent(text) + TextContent(source) } } From f24b593349bff0c471dccad4d7a78171e1c181a9 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Tue, 3 Nov 2020 15:56:51 +0300 Subject: [PATCH 2/3] Update MarkdownParserTest. --- .../sdk/internal/session/room/send/MarkdownParserTest.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 1713578932..1c739a9bae 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 @@ -25,6 +25,8 @@ import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.MethodSorters import org.matrix.android.sdk.InstrumentedTest +import org.matrix.android.sdk.internal.session.room.send.pills.MentionLinkSpecComparator +import org.matrix.android.sdk.internal.session.room.send.pills.TextPillsUtils /** * It will not be possible to test all combinations. For the moment I add a few tests, then, depending on the problem discovered in the wild, @@ -45,7 +47,8 @@ class MarkdownParserTest : InstrumentedTest { */ private val markdownParser = MarkdownParser( Parser.builder().build(), - HtmlRenderer.builder().build() + HtmlRenderer.builder().build(), + TextPillsUtils(MentionLinkSpecComparator()) ) @Test From bbc3dc0504022edd299e95ca79bcd4d320b845e5 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Wed, 4 Nov 2020 13:42:52 +0300 Subject: [PATCH 3/3] Set br softbreak to html parser. --- .../session/room/send/MarkdownParserTest.kt | 23 +++++++++++-------- .../sdk/internal/session/room/RoomModule.kt | 1 + 2 files changed, 14 insertions(+), 10 deletions(-) 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 1c739a9bae..94303dda08 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 @@ -47,7 +47,7 @@ class MarkdownParserTest : InstrumentedTest { */ private val markdownParser = MarkdownParser( Parser.builder().build(), - HtmlRenderer.builder().build(), + HtmlRenderer.builder().softbreak("
").build(), TextPillsUtils(MentionLinkSpecComparator()) ) @@ -147,12 +147,14 @@ class MarkdownParserTest : InstrumentedTest { ) } + // TODO. Improve testTypeNewLines function to cover 
test
@Test - fun parseCodeNewLines() { + fun parseCodeNewLines_not_passing() { testTypeNewLines( name = "code", - markdownPattern = "`", - htmlExpectedTag = "code" + markdownPattern = "```", + htmlExpectedTag = "code", + softBreak = "\n" ) } @@ -166,7 +168,7 @@ class MarkdownParserTest : InstrumentedTest { } @Test - fun parseCode2NewLines() { + fun parseCode2NewLines_not_passing() { testTypeNewLines( name = "code", markdownPattern = "``", @@ -184,7 +186,7 @@ class MarkdownParserTest : InstrumentedTest { } @Test - fun parseCode3NewLines() { + fun parseCode3NewLines_not_passing() { testTypeNewLines( name = "code", markdownPattern = "```", @@ -246,7 +248,7 @@ class MarkdownParserTest : InstrumentedTest { } @Test - fun parseBoldNewLines_not_passing() { + fun parseBoldNewLines2() { "**bold**\nline2".let { markdownParser.parse(it).expect(it, "bold
line2") } } @@ -337,13 +339,14 @@ class MarkdownParserTest : InstrumentedTest { private fun testTypeNewLines(name: String, markdownPattern: String, - htmlExpectedTag: String) { + htmlExpectedTag: String, + softBreak: String = "
") { // With new line inside the block "$markdownPattern$name\n$name$markdownPattern" .let { markdownParser.parse(it) .expect(expectedText = it, - expectedFormattedText = "<$htmlExpectedTag>$name
$name") + expectedFormattedText = "<$htmlExpectedTag>$name$softBreak$name") } // With new line between two blocks @@ -351,7 +354,7 @@ class MarkdownParserTest : InstrumentedTest { .let { markdownParser.parse(it) .expect(expectedText = it, - expectedFormattedText = "<$htmlExpectedTag>$name<$htmlExpectedTag>$name") + expectedFormattedText = "<$htmlExpectedTag>$name
<$htmlExpectedTag>$name") } } 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 30e3337a68..6381796ee0 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 @@ -101,6 +101,7 @@ internal abstract class RoomModule { fun providesHtmlRenderer(): HtmlRenderer { return HtmlRenderer .builder() + .softbreak("
") .build() } }