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 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/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt b/vector/src/androidTest/java/im/vector/app/features/html/SpanUtilsTest.kt index 2ede20a07d..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 @@ -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() @@ -92,5 +96,30 @@ class SpanUtilsTest : InstrumentedTest { spanUtils.canUseTextFuture(string) shouldBeEqualTo trueIfAlwaysAllowed() } + @Test + fun testGetBindingOptionsRegular() { + val string = SpannableString("Text") + val result = spanUtils.getBindingOptions(string) + result.canUseTextFuture shouldBeEqualTo true + result.preventMutation shouldBeEqualTo false + } + + @Test + fun testGetBindingOptionsStrikethrough() { + 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 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 } 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" /> + + + 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) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt index 414bf3cca1..3826c4cbad 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt @@ -37,6 +37,7 @@ import im.vector.app.features.home.room.detail.timeline.image.buildImageContentR import im.vector.app.features.home.room.detail.timeline.item.E2EDecoration import im.vector.app.features.home.room.detail.timeline.tools.createLinkMovementMethod import im.vector.app.features.home.room.detail.timeline.tools.linkify +import im.vector.app.features.html.SpanUtils import im.vector.app.features.media.ImageContentRenderer import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.failure.Failure @@ -53,6 +54,7 @@ class MessageActionsEpoxyController @Inject constructor( private val imageContentRenderer: ImageContentRenderer, private val dimensionConverter: DimensionConverter, private val errorFormatter: ErrorFormatter, + private val spanUtils: SpanUtils, private val eventDetailsFormatter: EventDetailsFormatter, private val dateFormatter: VectorDateFormatter ) : TypedEpoxyController() { @@ -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 35d92b0a23..22d282d567 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 @@ -497,7 +498,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 { @@ -509,7 +510,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) @@ -540,7 +541,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) @@ -589,7 +590,7 @@ class MessageItemFactory @Inject constructor( textStyle = "italic" } - val canUseTextFuture = spanUtils.canUseTextFuture(htmlBody) + val bindingOptions = spanUtils.getBindingOptions(htmlBody) val message = formattedBody.linkify(callback) return MessageTextItem_() @@ -599,7 +600,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..e50f141af1 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/BindingOptions.kt @@ -0,0 +1,24 @@ +/* + * 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( + // 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/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/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) 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..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 @@ -18,24 +18,43 @@ 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.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() + } + + return BindingOptions( + canUseTextFuture = canUseTextFuture(emojiCharSequence), + preventMutation = mustPreventMutation(emojiCharSequence) + ) + } + // Workaround for https://issuetracker.google.com/issues/188454876 - fun canUseTextFuture(charSequence: CharSequence): Boolean { + private fun canUseTextFuture(spanned: Spanned): 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 spanned + .getSpans(0, spanned.length, Any::class.java) + .all { it !is StrikethroughSpan && it !is UnderlineSpan && it !is MetricAffectingSpan } + } - return charSequence - .getSpans(0, charSequence.length, Any::class.java) - .all { it !is StrikethroughSpan && it !is UnderlineSpan } + // Workaround for setting text during binding which mutate the text itself + private fun mustPreventMutation(spanned: Spanned): Boolean { + return spanned + .getSpans(0, spanned.length, Any::class.java) + .any { it is MetricAffectingSpan } } }