From d904ebbd70d9a122ca8824636ea1cb6851d54717 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 5 Jul 2022 14:27:42 +0100 Subject: [PATCH 1/5] overriding the default list handler with an implementation that takes into account the initial starting position --- .../app/features/html/EventHtmlRenderer.kt | 2 + .../html/ListHandlerWithInitialStart.java | 111 ++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 vector/src/main/java/im/vector/app/features/html/ListHandlerWithInitialStart.java diff --git a/vector/src/main/java/im/vector/app/features/html/EventHtmlRenderer.kt b/vector/src/main/java/im/vector/app/features/html/EventHtmlRenderer.kt index ed6b68e04a..768e0f222d 100644 --- a/vector/src/main/java/im/vector/app/features/html/EventHtmlRenderer.kt +++ b/vector/src/main/java/im/vector/app/features/html/EventHtmlRenderer.kt @@ -152,7 +152,9 @@ class EventHtmlRenderer @Inject constructor( class MatrixHtmlPluginConfigure @Inject constructor(private val colorProvider: ColorProvider, private val resources: Resources) : HtmlPlugin.HtmlConfigure { override fun configureHtml(plugin: HtmlPlugin) { + plugin + .addHandler(ListHandlerWithInitialStart()) .addHandler(FontTagHandler()) .addHandler(ParagraphHandler(DimensionConverter(resources))) .addHandler(MxReplyTagHandler()) diff --git a/vector/src/main/java/im/vector/app/features/html/ListHandlerWithInitialStart.java b/vector/src/main/java/im/vector/app/features/html/ListHandlerWithInitialStart.java new file mode 100644 index 0000000000..c7ba881da0 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/html/ListHandlerWithInitialStart.java @@ -0,0 +1,111 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.html; + +import androidx.annotation.NonNull; + +import org.commonmark.node.ListItem; + +import java.util.Arrays; +import java.util.Collection; + +import io.noties.markwon.MarkwonConfiguration; +import io.noties.markwon.MarkwonVisitor; +import io.noties.markwon.RenderProps; +import io.noties.markwon.SpanFactory; +import io.noties.markwon.SpannableBuilder; +import io.noties.markwon.core.CoreProps; +import io.noties.markwon.html.HtmlTag; +import io.noties.markwon.html.MarkwonHtmlRenderer; +import io.noties.markwon.html.TagHandler; + +/** + * Copied from https://github.com/noties/Markwon/blob/master/markwon-html/src/main/java/io/noties/markwon/html/tag/ListHandler.java#L44 + * With a modification on the starting list position + */ +public class ListHandlerWithInitialStart extends TagHandler { + + private static final String START_KEY = "start"; + + @Override + public void handle( + @NonNull MarkwonVisitor visitor, + @NonNull MarkwonHtmlRenderer renderer, + @NonNull HtmlTag tag) { + + if (!tag.isBlock()) { + return; + } + + final HtmlTag.Block block = tag.getAsBlock(); + final boolean ol = "ol".equals(block.name()); + final boolean ul = "ul".equals(block.name()); + + if (!ol && !ul) { + return; + } + + final MarkwonConfiguration configuration = visitor.configuration(); + final RenderProps renderProps = visitor.renderProps(); + final SpanFactory spanFactory = configuration.spansFactory().get(ListItem.class); + + // Modified line + int number = Integer.parseInt(block.attributes().containsKey(START_KEY) ? block.attributes().get(START_KEY) : "1"); + + final int bulletLevel = currentBulletListLevel(block); + + for (HtmlTag.Block child : block.children()) { + + visitChildren(visitor, renderer, child); + + if (spanFactory != null && "li".equals(child.name())) { + + // insert list item here + if (ol) { + CoreProps.LIST_ITEM_TYPE.set(renderProps, CoreProps.ListItemType.ORDERED); + CoreProps.ORDERED_LIST_ITEM_NUMBER.set(renderProps, number++); + } else { + CoreProps.LIST_ITEM_TYPE.set(renderProps, CoreProps.ListItemType.BULLET); + CoreProps.BULLET_LIST_ITEM_LEVEL.set(renderProps, bulletLevel); + } + + SpannableBuilder.setSpans( + visitor.builder(), + spanFactory.getSpans(configuration, renderProps), + child.start(), + child.end()); + } + } + } + + @NonNull + @Override + public Collection supportedTags() { + return Arrays.asList("ol", "ul"); + } + + private static int currentBulletListLevel(@NonNull HtmlTag.Block block) { + int level = 0; + while ((block = block.parent()) != null) { + if ("ul".equals(block.name()) + || "ol".equals(block.name())) { + level += 1; + } + } + return level; + } +} From ff2fc3e0a76706ebc9ff1e94c3726c7b2d5deec9 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 5 Jul 2022 15:29:58 +0100 Subject: [PATCH 2/5] adding changelog entry --- changelog.d/4777.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4777.bugfix diff --git a/changelog.d/4777.bugfix b/changelog.d/4777.bugfix new file mode 100644 index 0000000000..428101f649 --- /dev/null +++ b/changelog.d/4777.bugfix @@ -0,0 +1 @@ +Fixes numbered lists always starting from 1 From a0f86d270b699a311bc0b23634829379fc29d029 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 5 Jul 2022 15:38:45 +0100 Subject: [PATCH 3/5] removing extra line --- .../main/java/im/vector/app/features/html/EventHtmlRenderer.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/html/EventHtmlRenderer.kt b/vector/src/main/java/im/vector/app/features/html/EventHtmlRenderer.kt index 768e0f222d..412b28862a 100644 --- a/vector/src/main/java/im/vector/app/features/html/EventHtmlRenderer.kt +++ b/vector/src/main/java/im/vector/app/features/html/EventHtmlRenderer.kt @@ -152,7 +152,6 @@ class EventHtmlRenderer @Inject constructor( class MatrixHtmlPluginConfigure @Inject constructor(private val colorProvider: ColorProvider, private val resources: Resources) : HtmlPlugin.HtmlConfigure { override fun configureHtml(plugin: HtmlPlugin) { - plugin .addHandler(ListHandlerWithInitialStart()) .addHandler(FontTagHandler()) From cfb1e09d64c184e12787824ce67c7b0e93917ff0 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 6 Jul 2022 11:44:33 +0100 Subject: [PATCH 4/5] adding tests around the event html rendering - the test helper is a little hacky in order to covert the spans to something human readable --- .../java/im/vector/app/core/utils/TestSpan.kt | 91 +++++++++++++++++++ .../features/html/EventHtmlRendererTest.kt | 75 +++++++++++++++ 2 files changed, 166 insertions(+) create mode 100644 vector/src/androidTest/java/im/vector/app/core/utils/TestSpan.kt create mode 100644 vector/src/androidTest/java/im/vector/app/features/html/EventHtmlRendererTest.kt diff --git a/vector/src/androidTest/java/im/vector/app/core/utils/TestSpan.kt b/vector/src/androidTest/java/im/vector/app/core/utils/TestSpan.kt new file mode 100644 index 0000000000..9e23e76f0c --- /dev/null +++ b/vector/src/androidTest/java/im/vector/app/core/utils/TestSpan.kt @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.core.utils + +import android.graphics.Canvas +import android.graphics.Paint +import android.text.Layout +import android.text.Spannable +import androidx.core.text.getSpans +import im.vector.app.features.html.HtmlCodeSpan +import io.mockk.justRun +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import io.noties.markwon.core.spans.EmphasisSpan +import io.noties.markwon.core.spans.OrderedListItemSpan +import io.noties.markwon.core.spans.StrongEmphasisSpan + +fun Spannable.toTestSpan(): String { + var output = toString() + readSpansWithContent().forEach { + val tags = it.span.readTags() + val remappedContent = it.span.remapContent(source = this, originalContent = it.content) + output = output.replace(it.content, "${tags.open}$remappedContent${tags.close}") + } + return output +} + +private fun Spannable.readSpansWithContent() = getSpans().map { span -> + val start = getSpanStart(span) + val end = getSpanEnd(span) + SpanWithContent( + content = substring(start, end), + span = span + ) +}.reversed() + +private fun Any.readTags(): SpanTags { + return when (this::class) { + OrderedListItemSpan::class -> SpanTags("[list item]", "[/list item]") + HtmlCodeSpan::class -> SpanTags("[code]", "[/code]") + StrongEmphasisSpan::class -> SpanTags("[bold]", "[/bold]") + EmphasisSpan::class -> SpanTags("[italic]", "[/italic]") + else -> throw IllegalArgumentException("Unknown ${this::class}") + } +} + +private fun Any.remapContent(source: CharSequence, originalContent: String): String { + return when (this::class) { + OrderedListItemSpan::class -> { + val prefix = (this as OrderedListItemSpan).collectNumber(source) + "$prefix$originalContent" + } + else -> originalContent + } +} + +private fun OrderedListItemSpan.collectNumber(text: CharSequence): String { + val fakeCanvas = mockk() + val fakeLayout = mockk() + justRun { fakeCanvas.drawText(any(), any(), any(), any()) } + val paint = Paint() + drawLeadingMargin(fakeCanvas, paint, 0, 0, 0, 0, 0, text, 0, text.length - 1, true, fakeLayout) + val slot = slot() + verify { fakeCanvas.drawText(capture(slot), any(), any(), any()) } + return slot.captured +} + +private data class SpanTags( + val open: String, + val close: String, +) + +private data class SpanWithContent( + val content: String, + val span: Any +) diff --git a/vector/src/androidTest/java/im/vector/app/features/html/EventHtmlRendererTest.kt b/vector/src/androidTest/java/im/vector/app/features/html/EventHtmlRendererTest.kt new file mode 100644 index 0000000000..a1b74778a5 --- /dev/null +++ b/vector/src/androidTest/java/im/vector/app/features/html/EventHtmlRendererTest.kt @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.html + +import androidx.core.text.toSpannable +import androidx.test.platform.app.InstrumentationRegistry +import im.vector.app.core.resources.ColorProvider +import im.vector.app.core.utils.toTestSpan +import im.vector.app.features.settings.VectorPreferences +import io.mockk.every +import io.mockk.mockk +import org.amshove.kluent.shouldBeEqualTo +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import kotlin.text.Typography.nbsp + +@RunWith(JUnit4::class) +class EventHtmlRendererTest { + + private val context = InstrumentationRegistry.getInstrumentation().targetContext + private val fakeVectorPreferences = mockk().also { + every { it.latexMathsIsEnabled() } returns false + } + + private val renderer = EventHtmlRenderer( + MatrixHtmlPluginConfigure(ColorProvider(context), context.resources), + context, + fakeVectorPreferences + ) + + @Test + fun takesInitialListPositionIntoAccount() { + val result = """
  1. first entry
""".renderAsTestSpan() + + result shouldBeEqualTo "[list item]5.${nbsp}first entry[/list item]\n" + } + + @Test + fun doesNotProcessMarkdownWithinCodeBlocks() { + val result = """__italic__ **bold**""".renderAsTestSpan() + + result shouldBeEqualTo "[code]__italic__ **bold**[/code]" + } + + @Test + fun doesNotProcessMarkdownBoldAndItalic() { + val result = """__italic__ **bold**""".renderAsTestSpan() + + result shouldBeEqualTo "__italic__ **bold**" + } + + @Test + fun processesHtmlWithinCodeBlocks() { + val result = """italic bold""".renderAsTestSpan() + + result shouldBeEqualTo "[code][italic]italic[/italic] [bold]bold[/bold][/code]" + } + + private fun String.renderAsTestSpan() = renderer.render(this).toSpannable().toTestSpan() +} From ead8cec4a69155d0995098f4575fc79be9486dd2 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 7 Jul 2022 13:54:42 +0100 Subject: [PATCH 5/5] adding test case for showing html entities are processed --- .../im/vector/app/features/html/EventHtmlRendererTest.kt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/vector/src/androidTest/java/im/vector/app/features/html/EventHtmlRendererTest.kt b/vector/src/androidTest/java/im/vector/app/features/html/EventHtmlRendererTest.kt index a1b74778a5..41c0f51322 100644 --- a/vector/src/androidTest/java/im/vector/app/features/html/EventHtmlRendererTest.kt +++ b/vector/src/androidTest/java/im/vector/app/features/html/EventHtmlRendererTest.kt @@ -71,5 +71,12 @@ class EventHtmlRendererTest { result shouldBeEqualTo "[code][italic]italic[/italic] [bold]bold[/bold][/code]" } + @Test + fun processesHtmlEntities() { + val result = """& < > ' """".renderAsTestSpan() + + result shouldBeEqualTo """& < > ' """" + } + private fun String.renderAsTestSpan() = renderer.render(this).toSpannable().toTestSpan() }