Merge pull request #3373 from vector-im/feature/bma/del_tag

Implement a workaround to render <del> and <u> in the timeline (#1817)

CHANGES.md

@@ -11,6 +11,7 @@ Bugfix 🐛:
  - Space Invite by link not always displayed for public space (#3345)
  - Wrong copy in share space bottom sheet (#3346)
  - Fix a problem with database migration on nightly builds (#3335)
+ - Implement a workaround to render <del> and <u> in the timeline (#1817)
 
 Translations 🗣:
  -

vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt

@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) 2021 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 android.graphics.Color
+import android.os.Build
+import android.text.SpannableString
+import android.text.SpannableStringBuilder
+import android.text.Spanned
+import android.text.style.ForegroundColorSpan
+import android.text.style.StrikethroughSpan
+import android.text.style.UnderlineSpan
+import im.vector.app.InstrumentedTest
+import org.amshove.kluent.shouldBeEqualTo
+import org.amshove.kluent.shouldBeTrue
+import org.junit.FixMethodOrder
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.junit.runners.MethodSorters
+
+@RunWith(JUnit4::class)
+@FixMethodOrder(MethodSorters.JVM)
+class SpanUtilsTest : InstrumentedTest {
+
+    private val spanUtils = SpanUtils()
+
+    @Test
+    fun canUseTextFutureString() {
+        spanUtils.canUseTextFuture("test").shouldBeTrue()
+    }
+
+    @Test
+    fun canUseTextFutureCharSequenceOK() {
+        spanUtils.canUseTextFuture(SpannableStringBuilder().append("hello")).shouldBeTrue()
+    }
+
+    @Test
+    fun canUseTextFutureCharSequenceWithSpanOK() {
+        val string = SpannableString("Text with strikethrough, underline, red spans")
+        string.setSpan(ForegroundColorSpan(Color.RED), 36, 39, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE)
+
+        spanUtils.canUseTextFuture(string) shouldBeEqualTo true
+    }
+
+    @Test
+    fun canUseTextFutureCharSequenceWithSpanKOStrikethroughSpan() {
+        val string = SpannableString("Text with strikethrough, underline, red spans")
+        string.setSpan(StrikethroughSpan(), 10, 23, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE)
+
+        spanUtils.canUseTextFuture(string) shouldBeEqualTo trueIfAlwaysAllowed()
+    }
+
+    @Test
+    fun canUseTextFutureCharSequenceWithSpanKOUnderlineSpan() {
+        val string = SpannableString("Text with strikethrough, underline, red spans")
+        string.setSpan(UnderlineSpan(), 25, 34, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE)
+
+        spanUtils.canUseTextFuture(string) shouldBeEqualTo trueIfAlwaysAllowed()
+    }
+
+    @Test
+    fun canUseTextFutureCharSequenceWithSpanKOBoth() {
+        val string = SpannableString("Text with strikethrough, underline, red spans")
+        string.setSpan(StrikethroughSpan(), 10, 23, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE)
+        string.setSpan(UnderlineSpan(), 25, 34, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE)
+
+        spanUtils.canUseTextFuture(string) shouldBeEqualTo trueIfAlwaysAllowed()
+    }
+
+    @Test
+    fun canUseTextFutureCharSequenceWithSpanKOAll() {
+        val string = SpannableString("Text with strikethrough, underline, red spans")
+        string.setSpan(StrikethroughSpan(), 10, 23, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE)
+        string.setSpan(UnderlineSpan(), 25, 34, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE)
+        string.setSpan(ForegroundColorSpan(Color.RED), 36, 39, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE)
+
+        spanUtils.canUseTextFuture(string) shouldBeEqualTo trueIfAlwaysAllowed()
+    }
+
+    private fun trueIfAlwaysAllowed() = Build.VERSION.SDK_INT < Build.VERSION_CODES.P
+}

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

@@ -60,6 +60,7 @@ import im.vector.app.features.home.room.detail.timeline.tools.linkify
 import im.vector.app.features.html.CodeVisitor
 import im.vector.app.features.html.EventHtmlRenderer
 import im.vector.app.features.html.PillsPostProcessor
+import im.vector.app.features.html.SpanUtils
 import im.vector.app.features.html.VectorHtmlCompressor
 import im.vector.app.features.media.ImageContentRenderer
 import im.vector.app.features.media.VideoContentRenderer
@@ -109,6 +110,7 @@ class MessageItemFactory @Inject constructor(
         private val noticeItemFactory: NoticeItemFactory,
         private val avatarSizeProvider: AvatarSizeProvider,
         private val pillsPostProcessorFactory: PillsPostProcessor.Factory,
+        private val spanUtils: SpanUtils,
         private val session: Session) {
 
     // TODO inject this properly?
@@ -420,6 +422,7 @@ class MessageItemFactory @Inject constructor(
                                      highlight: Boolean,
                                      callback: TimelineEventController.Callback?,
                                      attributes: AbsMessageItem.Attributes): MessageTextItem? {
+        val canUseTextFuture = spanUtils.canUseTextFuture(body)
         val linkifiedBody = body.linkify(callback)
 
         return MessageTextItem_().apply {
@@ -431,6 +434,7 @@ class MessageItemFactory @Inject constructor(
             }
         }
                 .useBigFont(linkifiedBody.length <= MAX_NUMBER_OF_EMOJI_FOR_BIG_FONT * 2 && containsOnlyEmojis(linkifiedBody.toString()))
+                .canUseTextFuture(canUseTextFuture)
                 .searchForPills(isFormatted)
                 .previewUrlRetriever(callback?.getPreviewUrlRetriever())
                 .imageContentRenderer(imageContentRenderer)
@@ -503,12 +507,14 @@ class MessageItemFactory @Inject constructor(
                                        highlight: Boolean,
                                        callback: TimelineEventController.Callback?,
                                        attributes: AbsMessageItem.Attributes): MessageTextItem? {
+        val htmlBody = messageContent.getHtmlBody()
         val formattedBody = span {
-            text = messageContent.getHtmlBody()
+            text = htmlBody
             textColor = colorProvider.getColorFromAttribute(R.attr.riotx_text_secondary)
             textStyle = "italic"
         }
 
+        val canUseTextFuture = spanUtils.canUseTextFuture(htmlBody)
         val message = formattedBody.linkify(callback)
 
         return MessageTextItem_()
@@ -518,6 +524,7 @@ class MessageItemFactory @Inject constructor(
                 .previewUrlCallback(callback)
                 .attributes(attributes)
                 .message(message)
+                .canUseTextFuture(canUseTextFuture)
                 .highlighted(highlight)
                 .movementMethod(createLinkMovementMethod(callback))
     }

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageTextItem.kt

@@ -40,6 +40,9 @@ abstract class MessageTextItem : AbsMessageItem<MessageTextItem.Holder>() {
     @EpoxyAttribute
     var message: CharSequence? = null
 
+    @EpoxyAttribute
+    var canUseTextFuture: Boolean = true
+
     @EpoxyAttribute
     var useBigFont: Boolean = false
 
@@ -80,17 +83,26 @@ abstract class MessageTextItem : AbsMessageItem<MessageTextItem.Holder>() {
                 it.bind(holder.messageView)
             }
         }
-        val textFuture = PrecomputedTextCompat.getTextFuture(
+        val textFuture = if (canUseTextFuture) {
-                message ?: "",
+            PrecomputedTextCompat.getTextFuture(
-                TextViewCompat.getTextMetricsParams(holder.messageView),
+                    message ?: "",
-                null)
+                    TextViewCompat.getTextMetricsParams(holder.messageView),
+                    null)
+        } else {
+            null
+        }
         super.bind(holder)
         holder.messageView.movementMethod = movementMethod
 
         renderSendState(holder.messageView, holder.messageView)
         holder.messageView.setOnClickListener(attributes.itemClickListener)
         holder.messageView.setOnLongClickListener(attributes.itemLongClickListener)
-        holder.messageView.setTextFuture(textFuture)
+
+        if (canUseTextFuture) {
+            holder.messageView.setTextFuture(textFuture)
+        } else {
+            holder.messageView.text = message
+        }
     }
 
     override fun unbind(holder: Holder) {

vector/src/main/java/im/vector/app/features/html/SpanUtils.kt

@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2021 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 android.os.Build
+import android.text.Spanned
+import android.text.style.StrikethroughSpan
+import android.text.style.UnderlineSpan
+import javax.inject.Inject
+
+class SpanUtils @Inject constructor() {
+    // Workaround for https://issuetracker.google.com/issues/188454876
+    fun canUseTextFuture(charSequence: CharSequence): Boolean {
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) {
+            // On old devices, it works correctly
+            return true
+        }
+
+        if (charSequence !is Spanned) {
+            return true
+        }
+
+        return charSequence
+                .getSpans(0, charSequence.length, Any::class.java)
+                .all { it !is StrikethroughSpan && it !is UnderlineSpan }
+    }
+}