From b701bb7c1a2b540fa11d0db9ca2cd99f7e0af326 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 13 Dec 2021 12:13:39 +0100 Subject: [PATCH 01/11] Fix crash when using TextFuture with MetricAffectingSpan added by EmojiCompat (#4691) --- .../java/im/vector/app/features/html/SpanUtils.kt | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt b/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt index 4e2c1c1a50..aab65e8880 100644 --- a/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt +++ b/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt @@ -18,8 +18,10 @@ package im.vector.app.features.html import android.os.Build import android.text.Spanned +import android.text.style.MetricAffectingSpan import android.text.style.StrikethroughSpan import android.text.style.UnderlineSpan +import androidx.emoji.text.EmojiCompat import javax.inject.Inject class SpanUtils @Inject constructor() { @@ -30,12 +32,13 @@ class SpanUtils @Inject constructor() { return true } - if (charSequence !is Spanned) { + val emojiCharSequence = EmojiCompat.get().process(charSequence) + if (emojiCharSequence !is Spanned) { return true } - return charSequence - .getSpans(0, charSequence.length, Any::class.java) - .all { it !is StrikethroughSpan && it !is UnderlineSpan } + return emojiCharSequence + .getSpans(0, emojiCharSequence.length, Any::class.java) + .all { it !is StrikethroughSpan && it !is UnderlineSpan && it !is MetricAffectingSpan } } } From df3f8bd88e476cee4481a04202b5b544346bb9e0 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 13 Dec 2021 12:36:46 +0100 Subject: [PATCH 02/11] Use emoji2 library --- vector/build.gradle | 2 +- vector/src/main/AndroidManifest.xml | 3 +++ vector/src/main/java/im/vector/app/EmojiCompatWrapper.kt | 6 +++--- .../src/main/java/im/vector/app/features/html/SpanUtils.kt | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/vector/build.gradle b/vector/build.gradle index 9f6c91c2b5..4c864f9309 100644 --- a/vector/build.gradle +++ b/vector/build.gradle @@ -449,7 +449,7 @@ dependencies { // OSS License, gplay flavor only gplayImplementation 'com.google.android.gms:play-services-oss-licenses:17.0.0' - implementation "androidx.emoji:emoji-appcompat:1.1.0" + implementation "androidx.emoji2:emoji2:1.0.0" implementation('com.github.BillCarsonFr:JsonViewer:0.7') // WebRTC diff --git a/vector/src/main/AndroidManifest.xml b/vector/src/main/AndroidManifest.xml index 6d3c6cdc51..034f9c7619 100644 --- a/vector/src/main/AndroidManifest.xml +++ b/vector/src/main/AndroidManifest.xml @@ -398,6 +398,9 @@ android:name="androidx.work.WorkManagerInitializer" android:value="androidx.startup" tools:node="remove" /> + + Date: Mon, 13 Dec 2021 13:14:20 +0100 Subject: [PATCH 03/11] Change usage of SpannableStringBuilder when this is not required --- .../home/room/detail/timeline/factory/MessageItemFactory.kt | 3 ++- .../home/room/detail/timeline/tools/EventRenderingTools.kt | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt index 98deaaf9c3..a97bd2f486 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt @@ -16,6 +16,7 @@ package im.vector.app.features.home.room.detail.timeline.factory +import android.text.Spannable import android.text.SpannableStringBuilder import android.text.Spanned import android.text.TextPaint @@ -515,7 +516,7 @@ class MessageItemFactory @Inject constructor( private fun annotateWithEdited(linkifiedBody: CharSequence, callback: TimelineEventController.Callback?, - informationData: MessageInformationData): SpannableStringBuilder { + informationData: MessageInformationData): Spannable { val spannable = SpannableStringBuilder() spannable.append(linkifiedBody) val editedSuffix = stringProvider.getString(R.string.edited_suffix) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/tools/EventRenderingTools.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/tools/EventRenderingTools.kt index 4abaa4fae4..1e8a81ff61 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/tools/EventRenderingTools.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/tools/EventRenderingTools.kt @@ -16,7 +16,6 @@ package im.vector.app.features.home.room.detail.timeline.tools -import android.text.SpannableStringBuilder import android.text.style.ClickableSpan import android.view.MotionEvent import android.widget.TextView @@ -45,7 +44,7 @@ fun CharSequence.findPillsAndProcess(scope: CoroutineScope, processBlock: (PillI fun CharSequence.linkify(callback: TimelineEventController.UrlClickCallback?): CharSequence { val text = this.toString() - val spannable = SpannableStringBuilder(this) + val spannable = toSpannable() MatrixLinkify.addLinks(spannable, object : MatrixPermalinkSpan.Callback { override fun onUrlClicked(url: String) { callback?.onUrlClicked(url, text) From 04d23ce7f62c751a701dcc21e3b6babe75e0f4b7 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 13 Dec 2021 14:56:21 +0100 Subject: [PATCH 04/11] Rename for clarity --- .../im/vector/app/core/linkify/VectorLinkify.kt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/linkify/VectorLinkify.kt b/vector/src/main/java/im/vector/app/core/linkify/VectorLinkify.kt index bc1ed3609b..0906b398c5 100644 --- a/vector/src/main/java/im/vector/app/core/linkify/VectorLinkify.kt +++ b/vector/src/main/java/im/vector/app/core/linkify/VectorLinkify.kt @@ -30,7 +30,7 @@ object VectorLinkify { if (keepExistingUrlSpan) { // Keep track of existing URLSpans, and mark them as important - spannable.forEachSpanIndexed { _, urlSpan, start, end -> + spannable.forEachUrlSpanIndexed { _, urlSpan, start, end -> createdSpans.add(LinkSpec(URLSpan(urlSpan.url), start, end, important = true)) } } @@ -39,7 +39,7 @@ object VectorLinkify { LinkifyCompat.addLinks(spannable, Linkify.WEB_URLS or Linkify.EMAIL_ADDRESSES or Linkify.PHONE_NUMBERS) // we might want to modify some matches - spannable.forEachSpanIndexed { _, urlSpan, start, end -> + spannable.forEachUrlSpanIndexed { _, urlSpan, start, end -> spannable.removeSpan(urlSpan) // remove short PN, too much false positive @@ -47,7 +47,7 @@ object VectorLinkify { if (end - start > 6) { // Do not match under 7 digit createdSpans.add(LinkSpec(URLSpan(urlSpan.url), start, end)) } - return@forEachSpanIndexed + return@forEachUrlSpanIndexed } // include mailto: if found before match @@ -60,7 +60,7 @@ object VectorLinkify { createdSpans.add(LinkSpec(URLSpan(urlSpan.url), start, end)) } - return@forEachSpanIndexed + return@forEachUrlSpanIndexed } // Handle url matches @@ -70,7 +70,7 @@ object VectorLinkify { // modify the span to include the slash val spec = LinkSpec(URLSpan(urlSpan.url + "/"), start, end + 1) createdSpans.add(spec) - return@forEachSpanIndexed + return@forEachUrlSpanIndexed } // Try to do something for ending ) issues/3020 if (spannable[end - 1] == ')') { @@ -87,7 +87,7 @@ object VectorLinkify { val span = URLSpan(spannable.substring(start, end - 1)) val spec = LinkSpec(span, start, end - 1) createdSpans.add(spec) - return@forEachSpanIndexed + return@forEachUrlSpanIndexed } } @@ -95,7 +95,7 @@ object VectorLinkify { } LinkifyCompat.addLinks(spannable, VectorAutoLinkPatterns.GEO_URI.toPattern(), "geo:", arrayOf("geo:"), geoMatchFilter, null) - spannable.forEachSpanIndexed { _, urlSpan, start, end -> + spannable.forEachUrlSpanIndexed { _, urlSpan, start, end -> spannable.removeSpan(urlSpan) createdSpans.add(LinkSpec(URLSpan(urlSpan.url), start, end)) } @@ -174,7 +174,7 @@ object VectorLinkify { return@MatchFilter true } - private inline fun Spannable.forEachSpanIndexed(action: (index: Int, urlSpan: URLSpan, start: Int, end: Int) -> Unit) { + private inline fun Spannable.forEachUrlSpanIndexed(action: (index: Int, urlSpan: URLSpan, start: Int, end: Int) -> Unit) { getSpans(0, length, URLSpan::class.java) .forEachIndexed { index, urlSpan -> val start = getSpanStart(urlSpan) From 808c401675c9328841340c87dea9c6f61ac0ca47 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 13 Dec 2021 18:15:19 +0100 Subject: [PATCH 05/11] Fix a crash on Epoxy if text contains a MetricAffectingSpan --- .../BottomSheetMessagePreviewItem.kt | 12 ++++++- .../vector/app/core/epoxy/util/Extensions.kt | 21 +++++++++++ .../action/MessageActionsEpoxyController.kt | 7 +++- .../timeline/factory/MessageItemFactory.kt | 8 ++--- .../detail/timeline/item/BindingOptions.kt | 22 ++++++++++++ .../detail/timeline/item/MessageTextItem.kt | 14 +++++--- .../im/vector/app/features/html/SpanUtils.kt | 35 ++++++++++++++----- 7 files changed, 101 insertions(+), 18 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/core/epoxy/util/Extensions.kt create mode 100644 vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/BindingOptions.kt diff --git a/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt b/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt index 417da4d2fe..eef057efd4 100644 --- a/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt +++ b/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt @@ -27,10 +27,13 @@ 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.onClick +import im.vector.app.core.epoxy.util.preventMutation import im.vector.app.core.extensions.setTextOrHide 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 /** @@ -48,6 +51,9 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel() { @@ -64,6 +66,8 @@ class MessageActionsEpoxyController @Inject constructor( // Message preview val date = state.timelineEvent()?.root?.originServerTs val formattedDate = dateFormatter.format(date, DateFormatKind.MESSAGE_DETAIL) + val body = state.messageBody.linkify(host.listener) + val bindingOptions = spanUtils.getBindingOptions(body) bottomSheetMessagePreviewItem { id("preview") avatarRenderer(host.avatarRenderer) @@ -72,7 +76,8 @@ class MessageActionsEpoxyController @Inject constructor( imageContentRenderer(host.imageContentRenderer) data(state.timelineEvent()?.buildImageContentRendererData(host.dimensionConverter.dpToPx(66))) userClicked { host.listener?.didSelectMenuAction(EventSharedAction.OpenUserProfile(state.informationData.senderId)) } - body(state.messageBody.linkify(host.listener)) + bindingOptions(bindingOptions) + body(body) bodyDetails(host.eventDetailsFormatter.format(state.timelineEvent()?.root)) time(formattedDate) } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt index a97bd2f486..c0e8c9c521 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt @@ -473,7 +473,7 @@ class MessageItemFactory @Inject constructor( highlight: Boolean, callback: TimelineEventController.Callback?, attributes: AbsMessageItem.Attributes): MessageTextItem? { - val canUseTextFuture = spanUtils.canUseTextFuture(body) + val bindingOptions = spanUtils.getBindingOptions(body) val linkifiedBody = body.linkify(callback) return MessageTextItem_().apply { @@ -485,7 +485,7 @@ class MessageItemFactory @Inject constructor( } } .useBigFont(linkifiedBody.length <= MAX_NUMBER_OF_EMOJI_FOR_BIG_FONT * 2 && containsOnlyEmojis(linkifiedBody.toString())) - .canUseTextFuture(canUseTextFuture) + .bindingOptions(bindingOptions) .searchForPills(isFormatted) .previewUrlRetriever(callback?.getPreviewUrlRetriever()) .imageContentRenderer(imageContentRenderer) @@ -565,7 +565,7 @@ class MessageItemFactory @Inject constructor( textStyle = "italic" } - val canUseTextFuture = spanUtils.canUseTextFuture(htmlBody) + val bindingOptions = spanUtils.getBindingOptions(htmlBody) val message = formattedBody.linkify(callback) return MessageTextItem_() @@ -575,7 +575,7 @@ class MessageItemFactory @Inject constructor( .previewUrlCallback(callback) .attributes(attributes) .message(message) - .canUseTextFuture(canUseTextFuture) + .bindingOptions(bindingOptions) .highlighted(highlight) .movementMethod(createLinkMovementMethod(callback)) } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/BindingOptions.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/BindingOptions.kt new file mode 100644 index 0000000000..317b8423a8 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/BindingOptions.kt @@ -0,0 +1,22 @@ +/* + * 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.home.room.detail.timeline.item + +data class BindingOptions( + val canUseTextFuture: Boolean, + val preventMutation: Boolean +) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageTextItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageTextItem.kt index 747183bce6..276008f09e 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageTextItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageTextItem.kt @@ -26,12 +26,14 @@ import com.airbnb.epoxy.EpoxyModelClass import im.vector.app.R import im.vector.app.core.epoxy.onClick import im.vector.app.core.epoxy.onLongClickIgnoringLinks +import im.vector.app.core.epoxy.util.preventMutation import im.vector.app.features.home.room.detail.timeline.TimelineEventController import im.vector.app.features.home.room.detail.timeline.tools.findPillsAndProcess import im.vector.app.features.home.room.detail.timeline.url.PreviewUrlRetriever import im.vector.app.features.home.room.detail.timeline.url.PreviewUrlUiState import im.vector.app.features.home.room.detail.timeline.url.PreviewUrlView import im.vector.app.features.media.ImageContentRenderer +import org.matrix.android.sdk.api.extensions.orFalse @EpoxyModelClass(layout = R.layout.item_timeline_event_base) abstract class MessageTextItem : AbsMessageItem() { @@ -43,7 +45,7 @@ abstract class MessageTextItem : AbsMessageItem() { var message: CharSequence? = null @EpoxyAttribute - var canUseTextFuture: Boolean = true + var bindingOptions: BindingOptions? = null @EpoxyAttribute var useBigFont: Boolean = false @@ -85,7 +87,7 @@ abstract class MessageTextItem : AbsMessageItem() { it.bind(holder.messageView) } } - val textFuture = if (canUseTextFuture) { + val textFuture = if (bindingOptions?.canUseTextFuture.orFalse()) { PrecomputedTextCompat.getTextFuture( message ?: "", TextViewCompat.getTextMetricsParams(holder.messageView), @@ -99,10 +101,14 @@ abstract class MessageTextItem : AbsMessageItem() { holder.messageView.onClick(attributes.itemClickListener) holder.messageView.onLongClickIgnoringLinks(attributes.itemLongClickListener) - if (canUseTextFuture) { + if (bindingOptions?.canUseTextFuture.orFalse()) { holder.messageView.setTextFuture(textFuture) } else { - holder.messageView.text = message + holder.messageView.text = if (bindingOptions?.preventMutation.orFalse()) { + message.preventMutation() + } else { + message + } } } diff --git a/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt b/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt index 9a786af5b4..804e245596 100644 --- a/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt +++ b/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt @@ -22,23 +22,42 @@ import android.text.style.MetricAffectingSpan import android.text.style.StrikethroughSpan import android.text.style.UnderlineSpan import androidx.emoji2.text.EmojiCompat +import im.vector.app.features.home.room.detail.timeline.item.BindingOptions import javax.inject.Inject class SpanUtils @Inject constructor() { + fun getBindingOptions(charSequence: CharSequence): BindingOptions { + val emojiCharSequence = EmojiCompat.get().process(charSequence) + + if (emojiCharSequence !is Spanned) { + return BindingOptions( + canUseTextFuture = true, + preventMutation = false + ) + } + + return BindingOptions( + canUseTextFuture = canUseTextFuture(emojiCharSequence), + preventMutation = preventMutation(emojiCharSequence) + ) + } + // Workaround for https://issuetracker.google.com/issues/188454876 - fun canUseTextFuture(charSequence: CharSequence): Boolean { + private fun canUseTextFuture(charSequence: Spanned): Boolean { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) { // On old devices, it works correctly return true } - val emojiCharSequence = EmojiCompat.get().process(charSequence) - if (emojiCharSequence !is Spanned) { - return true - } - - return emojiCharSequence - .getSpans(0, emojiCharSequence.length, Any::class.java) + return charSequence + .getSpans(0, charSequence.length, Any::class.java) .all { it !is StrikethroughSpan && it !is UnderlineSpan && it !is MetricAffectingSpan } } + + // Workaround for setting text during binding which mutate the text itself + private fun preventMutation(spanned: Spanned): Boolean { + return spanned + .getSpans(0, spanned.length, Any::class.java) + .any { it is MetricAffectingSpan } + } } From c302148e89b3fd4ce2d0b9254eebc6ae7a9b26e1 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 13 Dec 2021 18:21:27 +0100 Subject: [PATCH 06/11] Rename --- .../src/main/java/im/vector/app/features/html/SpanUtils.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt b/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt index 804e245596..175e595b37 100644 --- a/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt +++ b/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt @@ -43,14 +43,14 @@ class SpanUtils @Inject constructor() { } // Workaround for https://issuetracker.google.com/issues/188454876 - private fun canUseTextFuture(charSequence: Spanned): Boolean { + private fun canUseTextFuture(spanned: Spanned): Boolean { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) { // On old devices, it works correctly return true } - return charSequence - .getSpans(0, charSequence.length, Any::class.java) + return spanned + .getSpans(0, spanned.length, Any::class.java) .all { it !is StrikethroughSpan && it !is UnderlineSpan && it !is MetricAffectingSpan } } From e8fbbe2b977480e7ec0663a4e719ac28b9e87438 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 13 Dec 2021 20:09:33 +0100 Subject: [PATCH 07/11] Fix the test --- .../java/im/vector/app/features/html/SpanUtilsTest.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt b/vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt index 2ede20a07d..432098403f 100644 --- a/vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt +++ b/vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt @@ -39,6 +39,10 @@ class SpanUtilsTest : InstrumentedTest { private val spanUtils = SpanUtils() + private fun SpanUtils.canUseTextFuture(message: CharSequence): Boolean { + return getBindingOptions(message).canUseTextFuture + } + @Test fun canUseTextFutureString() { spanUtils.canUseTextFuture("test").shouldBeTrue() From 96b186ca8cc374a7cffb4a827a578b917b63310a Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 13 Dec 2021 20:28:58 +0100 Subject: [PATCH 08/11] Add tests to cover the new feature --- .../vector/app/features/html/SpanUtilsTest.kt | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt b/vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt index 432098403f..1aeeed678a 100644 --- a/vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt +++ b/vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt @@ -96,5 +96,30 @@ class SpanUtilsTest : InstrumentedTest { spanUtils.canUseTextFuture(string) shouldBeEqualTo trueIfAlwaysAllowed() } + @Test + fun `test get binding options regular`() { + val string = SpannableString("Text") + val result = spanUtils.getBindingOptions(string) + result.canUseTextFuture shouldBeEqualTo true + result.preventMutation shouldBeEqualTo false + } + + @Test + fun `test get binding options strikethrough`() { + val string = SpannableString("Text with strikethrough") + string.setSpan(StrikethroughSpan(), 10, 23, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE) + val result = spanUtils.getBindingOptions(string) + result.canUseTextFuture shouldBeEqualTo false + result.preventMutation shouldBeEqualTo false + } + + @Test + fun `test get binding options MetricAffectingSpan`() { + 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 } From 78a7a61d078efb98be9d33a4c8db38d446d60b15 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 13 Dec 2021 20:31:52 +0100 Subject: [PATCH 09/11] Changelog --- changelog.d/4698.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4698.bugfix diff --git a/changelog.d/4698.bugfix b/changelog.d/4698.bugfix new file mode 100644 index 0000000000..bcf20dd75f --- /dev/null +++ b/changelog.d/4698.bugfix @@ -0,0 +1 @@ +Fix a crash in the timeline with some Emojis. Also migrate to androidx.emoji2 \ No newline at end of file From 6e646b12b5520c355bb5bd8f468fd485937315b1 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 13 Dec 2021 20:34:44 +0100 Subject: [PATCH 10/11] Add some comments and default values --- .../home/room/detail/timeline/item/BindingOptions.kt | 6 ++++-- .../main/java/im/vector/app/features/html/SpanUtils.kt | 9 +++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/BindingOptions.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/BindingOptions.kt index 317b8423a8..e50f141af1 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/BindingOptions.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/BindingOptions.kt @@ -17,6 +17,8 @@ package im.vector.app.features.home.room.detail.timeline.item data class BindingOptions( - val canUseTextFuture: Boolean, - val preventMutation: Boolean + // Allowed by default + val canUseTextFuture: Boolean = true, + // No need to prevent mutation by default + val preventMutation: Boolean = false ) diff --git a/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt b/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt index 175e595b37..27c966e33e 100644 --- a/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt +++ b/vector/src/main/java/im/vector/app/features/html/SpanUtils.kt @@ -30,15 +30,12 @@ class SpanUtils @Inject constructor() { val emojiCharSequence = EmojiCompat.get().process(charSequence) if (emojiCharSequence !is Spanned) { - return BindingOptions( - canUseTextFuture = true, - preventMutation = false - ) + return BindingOptions() } return BindingOptions( canUseTextFuture = canUseTextFuture(emojiCharSequence), - preventMutation = preventMutation(emojiCharSequence) + preventMutation = mustPreventMutation(emojiCharSequence) ) } @@ -55,7 +52,7 @@ class SpanUtils @Inject constructor() { } // Workaround for setting text during binding which mutate the text itself - private fun preventMutation(spanned: Spanned): Boolean { + private fun mustPreventMutation(spanned: Spanned): Boolean { return spanned .getSpans(0, spanned.length, Any::class.java) .any { it is MetricAffectingSpan } From b9799b46fd0afaf5cc81371c7343117a1352fbc8 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 13 Dec 2021 22:45:24 +0100 Subject: [PATCH 11/11] Fix test compilation --- .../java/im/vector/app/features/html/SpanUtilsTest.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt b/vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt index 1aeeed678a..d33fd4948a 100644 --- a/vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt +++ b/vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt @@ -97,7 +97,7 @@ class SpanUtilsTest : InstrumentedTest { } @Test - fun `test get binding options regular`() { + fun testGetBindingOptionsRegular() { val string = SpannableString("Text") val result = spanUtils.getBindingOptions(string) result.canUseTextFuture shouldBeEqualTo true @@ -105,7 +105,7 @@ class SpanUtilsTest : InstrumentedTest { } @Test - fun `test get binding options strikethrough`() { + fun testGetBindingOptionsStrikethrough() { val string = SpannableString("Text with strikethrough") string.setSpan(StrikethroughSpan(), 10, 23, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE) val result = spanUtils.getBindingOptions(string) @@ -114,7 +114,7 @@ class SpanUtilsTest : InstrumentedTest { } @Test - fun `test get binding options MetricAffectingSpan`() { + fun testGetBindingOptionsMetricAffectingSpan() { val string = SpannableString("Emoji \uD83D\uDE2E\u200D\uD83D\uDCA8") val result = spanUtils.getBindingOptions(string) result.canUseTextFuture shouldBeEqualTo false