Merge pull request #4837 from vector-im/feature/bma/safe_epoxy_char_s…

…equence

Safe epoxy char sequence

changelog.d/4837.bugfix

@@ -0,0 +1 @@
+Stop using CharSequence as EpoxyAttribute because it can lead to crash if the CharSequence mutates during rendering.

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

@@ -108,7 +108,6 @@ class SpanUtilsTest : InstrumentedTest {
         val string = SpannableString("Text")
         val result = spanUtils.getBindingOptions(string)
         result.canUseTextFuture shouldBeEqualTo true
-        result.preventMutation shouldBeEqualTo false
     }
 
     @Test
@@ -117,15 +116,13 @@ class SpanUtilsTest : InstrumentedTest {
         string.setSpan(StrikethroughSpan(), 10, 23, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE)
         val result = spanUtils.getBindingOptions(string)
         result.canUseTextFuture shouldBeEqualTo false
-        result.preventMutation shouldBeEqualTo false
     }
 
     @Test
     fun testGetBindingOptionsMetricAffectingSpan() {
         val string = SpannableString("Emoji \uD83D\uDE2E\u200D\uD83D\uDCA8")
         val result = spanUtils.getBindingOptions(string)
         result.canUseTextFuture shouldBeEqualTo false
-        result.preventMutation shouldBeEqualTo true
     }
 
     private fun trueIfAlwaysAllowed() = Build.VERSION.SDK_INT < Build.VERSION_CODES.P

vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt

@@ -26,15 +26,14 @@ import im.vector.app.R
 import im.vector.app.core.epoxy.ClickListener
 import im.vector.app.core.epoxy.VectorEpoxyHolder
 import im.vector.app.core.epoxy.VectorEpoxyModel
+import im.vector.app.core.epoxy.charsequence.EpoxyCharSequence
 import im.vector.app.core.epoxy.onClick
-import im.vector.app.core.epoxy.util.preventMutation
 import im.vector.app.core.extensions.setTextOrHide
 import im.vector.app.features.displayname.getBestName
 import im.vector.app.features.home.AvatarRenderer
 import im.vector.app.features.home.room.detail.timeline.item.BindingOptions
 import im.vector.app.features.home.room.detail.timeline.tools.findPillsAndProcess
 import im.vector.app.features.media.ImageContentRenderer
-import org.matrix.android.sdk.api.extensions.orFalse
 import org.matrix.android.sdk.api.util.MatrixItem
 
 /**
@@ -50,13 +49,13 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel<BottomSheetMessa
     lateinit var matrixItem: MatrixItem
 
     @EpoxyAttribute
-    lateinit var body: CharSequence
+    lateinit var body: EpoxyCharSequence
 
     @EpoxyAttribute
     var bindingOptions: BindingOptions? = null
 
     @EpoxyAttribute
-    var bodyDetails: CharSequence? = null
+    var bodyDetails: EpoxyCharSequence? = null
 
     @EpoxyAttribute
     var imageContentRenderer: ImageContentRenderer? = null
@@ -65,7 +64,7 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel<BottomSheetMessa
     var data: ImageContentRenderer.Data? = null
 
     @EpoxyAttribute
-    var time: CharSequence? = null
+    var time: String? = null
 
     @EpoxyAttribute
     var movementMethod: MovementMethod? = null
@@ -84,13 +83,9 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel<BottomSheetMessa
         }
         holder.imagePreview.isVisible = data != null
         holder.body.movementMethod = movementMethod
-        holder.body.text = if (bindingOptions?.preventMutation.orFalse()) {
-            body.preventMutation()
-        } else {
-            body
-        }
-        holder.bodyDetails.setTextOrHide(bodyDetails)
-        body.findPillsAndProcess(coroutineScope) { it.bind(holder.body) }
+        holder.body.text = body.charSequence
+        holder.bodyDetails.setTextOrHide(bodyDetails?.charSequence)
+        body.charSequence.findPillsAndProcess(coroutineScope) { it.bind(holder.body) }
         holder.timestamp.setTextOrHide(time)
     }
 

vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetRadioActionItem.kt

@@ -37,7 +37,7 @@ import im.vector.app.core.extensions.setTextOrHide
 abstract class BottomSheetRadioActionItem : VectorEpoxyModel<BottomSheetRadioActionItem.Holder>() {
 
     @EpoxyAttribute
-    var title: CharSequence? = null
+    var title: String? = null
 
     @StringRes
     @EpoxyAttribute
@@ -47,7 +47,7 @@ abstract class BottomSheetRadioActionItem : VectorEpoxyModel<BottomSheetRadioAct
     var selected = false
 
     @EpoxyAttribute
-    var description: CharSequence? = null
+    var description: String? = null
 
     @EpoxyAttribute(EpoxyAttribute.Option.DoNotHash)
     lateinit var listener: ClickListener

vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetSendStateItem.kt

@@ -36,7 +36,7 @@ abstract class BottomSheetSendStateItem : VectorEpoxyModel<BottomSheetSendStateI
     var showProgress: Boolean = false
 
     @EpoxyAttribute
-    lateinit var text: CharSequence
+    lateinit var text: String
 
     @EpoxyAttribute
     @DrawableRes

vector/src/main/java/im/vector/app/core/epoxy/charsequence/EpoxyCharSequence.kt

@@ -0,0 +1,27 @@
+/*
+ * 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.epoxy.charsequence
+
+/**
+ * Wrapper for a CharSequence, which support mutation of the CharSequence, which can happen during rendering
+ */
+class EpoxyCharSequence(val charSequence: CharSequence) {
+    private val hash = charSequence.toString().hashCode()
+
+    override fun hashCode() = hash
+    override fun equals(other: Any?) = other is EpoxyCharSequence && other.hash == hash
+}

vector/src/main/java/im/vector/app/core/epoxy/util/Extensions.kt → vector/src/main/java/im/vector/app/core/epoxy/charsequence/Extensions.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 New Vector Ltd
+ * 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.
@@ -14,8 +14,9 @@
  * limitations under the License.
  */
 
-package im.vector.app.core.epoxy.util
+package im.vector.app.core.epoxy.charsequence
 
-import android.text.SpannableString
-
-fun CharSequence?.preventMutation(): CharSequence? = this?.let { SpannableString(it) }
+/**
+ * Extensions to wrap CharSequence to EpoxyCharSequence
+ */
+fun CharSequence.toEpoxyCharSequence() = EpoxyCharSequence(this)

vector/src/main/java/im/vector/app/core/epoxy/profiles/notifications/RadioButtonItem.kt

@@ -33,7 +33,7 @@ import im.vector.app.core.extensions.setAttributeTintedImageResource
 abstract class RadioButtonItem : VectorEpoxyModel<RadioButtonItem.Holder>() {
 
     @EpoxyAttribute
-    var title: CharSequence? = null
+    var title: String? = null
 
     @StringRes
     @EpoxyAttribute

vector/src/main/java/im/vector/app/core/ui/bottomsheet/BottomSheetGenericRadioAction.kt

@@ -23,7 +23,7 @@ import im.vector.app.core.platform.VectorSharedAction
  * Parent class for a bottom sheet action
  */
 open class BottomSheetGenericRadioAction(
-        open val title: CharSequence?,
+        open val title: String?,
         open val description: String? = null,
         open val isSelected: Boolean
 ) : VectorSharedAction {

vector/src/main/java/im/vector/app/core/ui/list/GenericEmptyWithActionItem.kt

@@ -39,10 +39,10 @@ import im.vector.app.core.extensions.setTextOrHide
 abstract class GenericEmptyWithActionItem : VectorEpoxyModel<GenericEmptyWithActionItem.Holder>() {
 
     @EpoxyAttribute
-    var title: CharSequence? = null
+    var title: String? = null
 
     @EpoxyAttribute
-    var description: CharSequence? = null
+    var description: String? = null
 
     @EpoxyAttribute
     @DrawableRes

vector/src/main/java/im/vector/app/core/ui/list/GenericFooterItem.kt

@@ -38,7 +38,7 @@ import im.vector.app.features.themes.ThemeUtils
 abstract class GenericFooterItem : VectorEpoxyModel<GenericFooterItem.Holder>() {
 
     @EpoxyAttribute
-    var text: CharSequence? = null
+    var text: String? = null
 
     @EpoxyAttribute
     var style: ItemStyle = ItemStyle.NORMAL_TEXT

vector/src/main/java/im/vector/app/core/ui/list/GenericItem.kt

@@ -28,6 +28,7 @@ import im.vector.app.R
 import im.vector.app.core.epoxy.ClickListener
 import im.vector.app.core.epoxy.VectorEpoxyHolder
 import im.vector.app.core.epoxy.VectorEpoxyModel
+import im.vector.app.core.epoxy.charsequence.EpoxyCharSequence
 import im.vector.app.core.epoxy.onClick
 import im.vector.app.core.extensions.setTextOrHide
 
@@ -41,10 +42,10 @@ import im.vector.app.core.extensions.setTextOrHide
 abstract class GenericItem : VectorEpoxyModel<GenericItem.Holder>() {
 
     @EpoxyAttribute
-    var title: CharSequence? = null
+    var title: EpoxyCharSequence? = null
 
     @EpoxyAttribute
-    var description: CharSequence? = null
+    var description: EpoxyCharSequence? = null
 
     @EpoxyAttribute
     var style: ItemStyle = ItemStyle.NORMAL_TEXT
@@ -71,7 +72,7 @@ abstract class GenericItem : VectorEpoxyModel<GenericItem.Holder>() {
 
     override fun bind(holder: Holder) {
         super.bind(holder)
-        holder.titleText.setTextOrHide(title)
+        holder.titleText.setTextOrHide(title?.charSequence)
 
         if (titleIconResourceId != -1) {
             holder.titleIcon.setImageResource(titleIconResourceId)
@@ -82,7 +83,7 @@ abstract class GenericItem : VectorEpoxyModel<GenericItem.Holder>() {
 
         holder.titleText.textSize = style.toTextSize()
 
-        holder.descriptionText.setTextOrHide(description)
+        holder.descriptionText.setTextOrHide(description?.charSequence)
 
         if (hasIndeterminateProcess) {
             holder.progressBar.isVisible = true

vector/src/main/java/im/vector/app/core/ui/list/GenericPillItem.kt

@@ -28,6 +28,7 @@ import im.vector.app.R
 import im.vector.app.core.epoxy.ClickListener
 import im.vector.app.core.epoxy.VectorEpoxyHolder
 import im.vector.app.core.epoxy.VectorEpoxyModel
+import im.vector.app.core.epoxy.charsequence.EpoxyCharSequence
 import im.vector.app.core.epoxy.onClick
 import im.vector.app.core.extensions.setTextOrHide
 import im.vector.app.features.themes.ThemeUtils
@@ -39,7 +40,7 @@ import im.vector.app.features.themes.ThemeUtils
 abstract class GenericPillItem : VectorEpoxyModel<GenericPillItem.Holder>() {
 
     @EpoxyAttribute
-    var text: CharSequence? = null
+    var text: EpoxyCharSequence? = null
 
     @EpoxyAttribute
     var style: ItemStyle = ItemStyle.NORMAL_TEXT
@@ -60,7 +61,7 @@ abstract class GenericPillItem : VectorEpoxyModel<GenericPillItem.Holder>() {
     override fun bind(holder: Holder) {
         super.bind(holder)
 
-        holder.textView.setTextOrHide(text)
+        holder.textView.setTextOrHide(text?.charSequence)
         holder.textView.typeface = style.toTypeFace()
         holder.textView.textSize = style.toTextSize()
         holder.textView.gravity = if (centered) Gravity.CENTER_HORIZONTAL else Gravity.START

vector/src/main/java/im/vector/app/core/ui/list/GenericWithValueItem.kt

@@ -27,6 +27,7 @@ import im.vector.app.R
 import im.vector.app.core.epoxy.ClickListener
 import im.vector.app.core.epoxy.VectorEpoxyHolder
 import im.vector.app.core.epoxy.VectorEpoxyModel
+import im.vector.app.core.epoxy.charsequence.EpoxyCharSequence
 import im.vector.app.core.epoxy.onClick
 import im.vector.app.core.extensions.setTextOrHide
 import im.vector.app.features.themes.ThemeUtils
@@ -41,10 +42,10 @@ import im.vector.app.features.themes.ThemeUtils
 abstract class GenericWithValueItem : VectorEpoxyModel<GenericWithValueItem.Holder>() {
 
     @EpoxyAttribute
-    var title: CharSequence? = null
+    var title: EpoxyCharSequence? = null
 
     @EpoxyAttribute
-    var value: CharSequence? = null
+    var value: String? = null
 
     @EpoxyAttribute
     @ColorInt
@@ -62,7 +63,7 @@ abstract class GenericWithValueItem : VectorEpoxyModel<GenericWithValueItem.Hold
 
     override fun bind(holder: Holder) {
         super.bind(holder)
-        holder.titleText.setTextOrHide(title)
+        holder.titleText.setTextOrHide(title?.charSequence)
 
         if (titleIconResourceId != -1) {
             holder.titleIcon.setImageResource(titleIconResourceId)

vector/src/main/java/im/vector/app/features/autocomplete/command/AutocompleteCommandItem.kt

@@ -29,13 +29,13 @@ import im.vector.app.core.epoxy.onClick
 abstract class AutocompleteCommandItem : VectorEpoxyModel<AutocompleteCommandItem.Holder>() {
 
     @EpoxyAttribute
-    var name: CharSequence? = null
+    var name: String? = null
 
     @EpoxyAttribute
-    var parameters: CharSequence? = null
+    var parameters: String? = null
 
     @EpoxyAttribute
-    var description: CharSequence? = null
+    var description: String? = null
 
     @EpoxyAttribute(EpoxyAttribute.Option.DoNotHash)
     var clickListener: ClickListener? = null