Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emoji fix #4698

Merged
merged 11 commits into from
Dec 13, 2021
Merged
1 change: 1 addition & 0 deletions changelog.d/4698.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a crash in the timeline with some Emojis. Also migrate to androidx.emoji2
2 changes: 1 addition & 1 deletion vector/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
}
3 changes: 3 additions & 0 deletions vector/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions vector/src/main/java/im/vector/app/EmojiCompatWrapper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand All @@ -48,6 +51,9 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel<BottomSheetMessa
@EpoxyAttribute
lateinit var body: CharSequence

@EpoxyAttribute
var bindingOptions: BindingOptions? = null

@EpoxyAttribute
var bodyDetails: CharSequence? = null

Expand Down Expand Up @@ -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)
Expand Down
21 changes: 21 additions & 0 deletions vector/src/main/java/im/vector/app/core/epoxy/util/Extensions.kt
Original file line number Diff line number Diff line change
@@ -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) }
16 changes: 8 additions & 8 deletions vector/src/main/java/im/vector/app/core/linkify/VectorLinkify.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
Expand All @@ -39,15 +39,15 @@ 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
if (urlSpan.url?.startsWith("tel:") == true) {
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
Expand All @@ -60,7 +60,7 @@ object VectorLinkify {
createdSpans.add(LinkSpec(URLSpan(urlSpan.url), start, end))
}

return@forEachSpanIndexed
return@forEachUrlSpanIndexed
}

// Handle url matches
Expand All @@ -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] == ')') {
Expand All @@ -87,15 +87,15 @@ object VectorLinkify {
val span = URLSpan(spannable.substring(start, end - 1))
val spec = LinkSpec(span, start, end - 1)
createdSpans.add(spec)
return@forEachSpanIndexed
return@forEachUrlSpanIndexed
}
}

createdSpans.add(LinkSpec(URLSpan(urlSpan.url), start, end))
}

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))
}
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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>() {
Expand All @@ -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)
Expand All @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -472,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 {
Expand All @@ -484,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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -564,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_()
Expand All @@ -574,7 +575,7 @@ class MessageItemFactory @Inject constructor(
.previewUrlCallback(callback)
.attributes(attributes)
.message(message)
.canUseTextFuture(canUseTextFuture)
.bindingOptions(bindingOptions)
.highlighted(highlight)
.movementMethod(createLinkMovementMethod(callback))
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
)
Original file line number Diff line number Diff line change
Expand Up @@ -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>() {
Expand All @@ -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
Expand Down Expand Up @@ -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),
Expand All @@ -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
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Loading