Merge pull request #4698 from vector-im/feature/bma/emoji_hotfix

Emoji fix
This commit is contained in:
Benoit Marty 2021-12-13 23:15:46 +01:00 committed by GitHub
commit 48fa411d83
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 150 additions and 32 deletions

1
changelog.d/4698.bugfix Normal file
View file

@ -0,0 +1 @@
Fix a crash in the timeline with some Emojis. Also migrate to androidx.emoji2

View file

@ -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

View file

@ -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
}

View file

@ -398,6 +398,9 @@
android:name="androidx.work.WorkManagerInitializer"
android:value="androidx.startup"
tools:node="remove" />
<!-- We init the lib ourself in EmojiCompatWrapper -->
<meta-data android:name="androidx.emoji2.text.EmojiCompatInitializer"
tools:node="remove" />
</provider>
<provider

View file

@ -17,8 +17,8 @@ package im.vector.app
import android.content.Context
import androidx.core.provider.FontRequest
import androidx.emoji.text.EmojiCompat
import androidx.emoji.text.FontRequestEmojiCompatConfig
import androidx.emoji2.text.EmojiCompat
import androidx.emoji2.text.FontRequestEmojiCompatConfig
import timber.log.Timber
import javax.inject.Inject
import javax.inject.Singleton
@ -52,7 +52,7 @@ class EmojiCompatWrapper @Inject constructor(private val context: Context) {
fun safeEmojiSpanify(sequence: CharSequence): CharSequence {
if (initialized) {
try {
return EmojiCompat.get().process(sequence)
return EmojiCompat.get().process(sequence) ?: sequence
} catch (throwable: Throwable) {
// Defensive coding against error (should not happend as it is initialized)
Timber.e(throwable, "Failed to init EmojiCompat")

View file

@ -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<BottomSheetMessa
@EpoxyAttribute
lateinit var body: CharSequence
@EpoxyAttribute
var bindingOptions: BindingOptions? = null
@EpoxyAttribute
var bodyDetails: CharSequence? = null
@ -77,7 +83,11 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel<BottomSheetMessa
}
holder.imagePreview.isVisible = data != null
holder.body.movementMethod = movementMethod
holder.body.text = body
holder.body.text = if (bindingOptions?.preventMutation.orFalse()) {
body.preventMutation()
} else {
body
}
holder.bodyDetails.setTextOrHide(bodyDetails)
body.findPillsAndProcess(coroutineScope) { it.bind(holder.body) }
holder.timestamp.setTextOrHide(time)

View file

@ -0,0 +1,21 @@
/*
* 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.core.epoxy.util
import android.text.SpannableString
fun CharSequence?.preventMutation(): CharSequence? = this?.let { SpannableString(it) }

View file

@ -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)

View file

@ -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<MessageActionState>() {
@ -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)
}

View file

@ -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))
}

View file

@ -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
)

View file

@ -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<MessageTextItem.Holder>() {
@ -43,7 +45,7 @@ abstract class MessageTextItem : AbsMessageItem<MessageTextItem.Holder>() {
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<MessageTextItem.Holder>() {
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<MessageTextItem.Holder>() {
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
}
}
}

View file

@ -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)

View file

@ -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 }
}
}