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.

Sign up for GitHub

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

Fix a bunch of issues related to edition and reply #5969 #8716

Merged
merged 3 commits into from
Jan 2, 2024
Merged

Fix a bunch of issues related to edition and reply #5969 #8716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b72039e
Fix a bunch of issues related to edition and reply #5969
ganfra Dec 20, 2023
320aae1
Also fix pills with rich text editor
ganfra Dec 20, 2023
2ada4c8
Fix quality
ganfra Dec 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/5969.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix some issues related to edition and reply of events.
2 changes: 1 addition & 1 deletion ...d/src/main/java/org/matrix/android/sdk/api/session/room/model/relation/RelationService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ interface RelationService {
fun editReply(
replyToEdit: TimelineEvent,
originalTimelineEvent: TimelineEvent,
newBodyText: String,
newBodyText: CharSequence,
newFormattedBodyText: String? = null,
compatibilityBodyText: String = "* $newBodyText"
): Cancelable
Expand Down
40 changes: 10 additions & 30 deletions ...k-android/src/main/java/org/matrix/android/sdk/api/session/room/timeline/TimelineEvent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ import org.matrix.android.sdk.api.session.room.model.message.MessageBeaconLocati
import org.matrix.android.sdk.api.session.room.model.message.MessageContent
import org.matrix.android.sdk.api.session.room.model.message.MessageContentWithFormattedBody
import org.matrix.android.sdk.api.session.room.model.message.MessageEndPollContent
import org.matrix.android.sdk.api.session.room.model.message.MessageFormat
import org.matrix.android.sdk.api.session.room.model.message.MessagePollContent
import org.matrix.android.sdk.api.session.room.model.message.MessageStickerContent
import org.matrix.android.sdk.api.session.room.model.message.MessageTextContent
import org.matrix.android.sdk.api.session.room.model.relation.RelationDefaultContent
import org.matrix.android.sdk.api.session.room.sender.SenderInfo
import org.matrix.android.sdk.api.util.ContentUtils
import org.matrix.android.sdk.api.util.ContentUtils.ensureCorrectFormattedBodyInTextReply
import org.matrix.android.sdk.api.util.ContentUtils.extractUsefulTextFromReply

/**
Expand Down Expand Up @@ -160,37 +160,17 @@ fun TimelineEvent.getLastMessageContent(): MessageContent? {

fun TimelineEvent.getLastEditNewContent(): Content? {
val lastContent = annotations?.editSummary?.latestEdit?.getClearContent()?.toModel<MessageContent>()?.newContent
return if (isReply()) {
val previousFormattedBody = root.getClearContent().toModel<MessageTextContent>()?.formattedBody
if (previousFormattedBody?.isNotEmpty() == true) {
val lastMessageContent = lastContent.toModel<MessageTextContent>()
lastMessageContent?.let { ensureCorrectFormattedBodyInTextReply(it, previousFormattedBody) }?.toContent() ?: lastContent
} else {
lastContent
}
} else {
lastContent
}
}

private const val MX_REPLY_END_TAG = "</mx-reply>"

/**
* Not every client sends a formatted body in the last edited event since this is not required in the
* [Matrix specification](https://spec.matrix.org/v1.4/client-server-api/#applying-mnew_content).
* We must ensure there is one so that it is still considered as a reply when rendering the message.
*/
private fun ensureCorrectFormattedBodyInTextReply(messageTextContent: MessageTextContent, previousFormattedBody: String): MessageTextContent {
return when {
messageTextContent.formattedBody.isNullOrEmpty() && previousFormattedBody.contains(MX_REPLY_END_TAG) -> {
// take previous formatted body with the new body content
val newFormattedBody = previousFormattedBody.replaceAfterLast(MX_REPLY_END_TAG, messageTextContent.body)
messageTextContent.copy(
formattedBody = newFormattedBody,
format = MessageFormat.FORMAT_MATRIX_HTML,
)
isReply() -> {
val originalFormattedBody = root.getClearContent().toModel<MessageTextContent>()?.formattedBody
val lastMessageContent = lastContent.toModel<MessageTextContent>()
if (lastMessageContent != null && originalFormattedBody?.isNotEmpty() == true) {
ensureCorrectFormattedBodyInTextReply(lastMessageContent, originalFormattedBody).toContent()
} else {
lastContent
}
}
else -> messageTextContent
else -> lastContent
}
}

Expand Down
31 changes: 28 additions & 3 deletions matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/util/ContentUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package org.matrix.android.sdk.api.util

import org.matrix.android.sdk.api.session.room.model.message.MessageFormat
import org.matrix.android.sdk.api.session.room.model.message.MessageTextContent
import org.matrix.android.sdk.internal.util.unescapeHtml

object ContentUtils {
Expand All @@ -38,15 +40,36 @@ object ContentUtils {
}

fun extractUsefulTextFromHtmlReply(repliedBody: String): String {
if (repliedBody.startsWith("<mx-reply>")) {
val closingTagIndex = repliedBody.lastIndexOf("</mx-reply>")
if (repliedBody.startsWith(MX_REPLY_START_TAG)) {
val closingTagIndex = repliedBody.lastIndexOf(MX_REPLY_END_TAG)
if (closingTagIndex != -1) {
return repliedBody.substring(closingTagIndex + "</mx-reply>".length).trim()
return repliedBody.substring(closingTagIndex + MX_REPLY_END_TAG.length).trim()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we get a </ mx-reply> tag, with one or several whitespaces?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope not, I kept the same tags than before.

}
}
return repliedBody
}

/**
* Not every client sends a formatted body in the last edited event since this is not required in the
* [Matrix specification](https://spec.matrix.org/v1.4/client-server-api/#applying-mnew_content).
* We must ensure there is one so that it is still considered as a reply when rendering the message.
*/
fun ensureCorrectFormattedBodyInTextReply(messageTextContent: MessageTextContent, originalFormattedBody: String): MessageTextContent {
return when {
messageTextContent.formattedBody != null &&
!messageTextContent.formattedBody.contains(MX_REPLY_END_TAG) &&
originalFormattedBody.contains(MX_REPLY_END_TAG) -> {
// take previous formatted body with the new body content
val newFormattedBody = originalFormattedBody.replaceAfterLast(MX_REPLY_END_TAG, messageTextContent.body)
messageTextContent.copy(
formattedBody = newFormattedBody,
format = MessageFormat.FORMAT_MATRIX_HTML,
)
}
else -> messageTextContent
}
}

@Suppress("RegExpRedundantEscape")
fun formatSpoilerTextFromHtml(formattedBody: String): String {
// var reason = "",
Expand All @@ -57,4 +80,6 @@ object ContentUtils {
}

private const val SPOILER_CHAR = "█"
private const val MX_REPLY_START_TAG = "<mx-reply>"
private const val MX_REPLY_END_TAG = "</mx-reply>"
}
2 changes: 1 addition & 1 deletion ...main/java/org/matrix/android/sdk/internal/session/room/relation/DefaultRelationService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ internal class DefaultRelationService @AssistedInject constructor(
override fun editReply(
replyToEdit: TimelineEvent,
originalTimelineEvent: TimelineEvent,
newBodyText: String,
newBodyText: CharSequence,
newFormattedBodyText: String?,
compatibilityBodyText: String
): Cancelable {
Expand Down
3 changes: 2 additions & 1 deletion ...ndroid/src/main/java/org/matrix/android/sdk/internal/session/room/relation/EventEditor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ internal class EventEditor @Inject constructor(
fun editReply(
replyToEdit: TimelineEvent,
originalTimelineEvent: TimelineEvent,
newBodyText: String,
newBodyText: CharSequence,
newBodyFormattedText: String?,
compatibilityBodyText: String
): Cancelable {
Expand All @@ -131,6 +131,7 @@ internal class EventEditor @Inject constructor(
replyToEdit,
originalTimelineEvent,
newBodyText,
newBodyFormattedText,
true,
MessageType.MSGTYPE_TEXT,
compatibilityBodyText
Expand Down
14 changes: 8 additions & 6 deletions .../src/main/java/org/matrix/android/sdk/internal/session/room/send/LocalEchoEventFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,8 @@ internal class LocalEchoEventFactory @Inject constructor(
roomId: String,
eventReplaced: TimelineEvent,
originalEvent: TimelineEvent,
newBodyText: String,
replyText: CharSequence,
replyTextFormatted: String?,
autoMarkdown: Boolean,
msgType: String,
compatibilityText: String,
Expand All @@ -321,22 +322,23 @@ internal class LocalEchoEventFactory @Inject constructor(
val permalink = permalinkFactory.createPermalink(roomId, originalEvent.root.eventId ?: "", false)
val userLink = originalEvent.root.senderId?.let { permalinkFactory.createPermalink(it, false) } ?: ""

val body = bodyForReply(timelineEvent = originalEvent)
val bodyOfRepliedEvent = bodyForReply(timelineEvent = originalEvent)
// As we always supply formatted body for replies we should force the MarkdownParser to produce html.
val newBodyFormatted = markdownParser.parse(newBodyText, force = true, advanced = autoMarkdown).takeFormatted()
val newBodyFormatted = replyTextFormatted ?: markdownParser.parse(replyText, force = true, advanced = autoMarkdown).takeFormatted()
// Body of the original message may not have formatted version, so may also have to convert to html.
val bodyFormatted = body.formattedText ?: markdownParser.parse(body.text, force = true, advanced = autoMarkdown).takeFormatted()
val formattedBodyOfRepliedEvent =
bodyOfRepliedEvent.formattedText ?: markdownParser.parse(text = bodyOfRepliedEvent.text, force = true, advanced = autoMarkdown).takeFormatted()
val replyFormatted = buildFormattedReply(
permalink,
userLink,
originalEvent.senderInfo.disambiguatedDisplayName,
bodyFormatted,
formattedBodyOfRepliedEvent,
newBodyFormatted
)
//
// > <@alice:example.org> This is the original body
//
val replyFallback = buildReplyFallback(body, originalEvent.root.senderId ?: "", newBodyText)
val replyFallback = buildReplyFallback(bodyOfRepliedEvent, originalEvent.root.senderId ?: "", replyText.toString())

return createMessageEvent(
roomId,
Expand Down
9 changes: 5 additions & 4 deletions ...rc/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ class MessageComposerViewModel @AssistedInject constructor(
private fun handleEnterEditMode(room: Room, action: MessageComposerAction.EnterEditMode) {
room.getTimelineEvent(action.eventId)?.let { timelineEvent ->
val formatted = vectorPreferences.isRichTextEditorEnabled()
setState { copy(sendMode = SendMode.Edit(timelineEvent, timelineEvent.getTextEditableContent(formatted))) }
val editableContent = timelineEvent.getTextEditableContent(formatted)
setState { copy(sendMode = SendMode.Edit(timelineEvent, editableContent)) }
}
}

Expand Down Expand Up @@ -578,7 +579,7 @@ class MessageComposerViewModel @AssistedInject constructor(
if (inReplyTo != null) {
// TODO check if same content?
room.getTimelineEvent(inReplyTo)?.let {
room.relationService().editReply(state.sendMode.timelineEvent, it, action.text.toString(), action.formattedText)
room.relationService().editReply(state.sendMode.timelineEvent, it, action.text, action.formattedText)
}
} else {
val messageContent = state.sendMode.timelineEvent.getVectorLastMessageContent()
Expand Down Expand Up @@ -624,14 +625,14 @@ class MessageComposerViewModel @AssistedInject constructor(
state.rootThreadEventId?.let {
room.relationService().replyInThread(
rootThreadEventId = it,
replyInThreadText = action.text.toString(),
replyInThreadText = action.text,
autoMarkdown = action.autoMarkdown,
formattedText = action.formattedText,
eventReplied = timelineEvent
)
} ?: room.relationService().replyToMessage(
eventReplied = timelineEvent,
replyText = action.text.toString(),
replyText = action.text,
replyFormattedText = action.formattedText,
autoMarkdown = action.autoMarkdown,
showInThread = showInThread,
Expand Down
8 changes: 7 additions & 1 deletion ...src/main/java/im/vector/app/features/home/room/detail/composer/PlainTextComposerLayout.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import org.matrix.android.sdk.api.session.room.model.message.MessageEndPollConte
import org.matrix.android.sdk.api.session.room.model.message.MessageFormat
import org.matrix.android.sdk.api.session.room.model.message.MessagePollContent
import org.matrix.android.sdk.api.session.room.model.message.MessageTextContent
import org.matrix.android.sdk.api.util.ContentUtils
import org.matrix.android.sdk.api.util.MatrixItem
import org.matrix.android.sdk.api.util.toMatrixItem
import javax.inject.Inject
Expand Down Expand Up @@ -188,7 +189,12 @@ class PlainTextComposerLayout @JvmOverloads constructor(
var formattedBody: CharSequence? = null
if (messageContent is MessageTextContent && messageContent.format == MessageFormat.FORMAT_MATRIX_HTML) {
val parser = Parser.builder().build()
val document = parser.parse(messageContent.formattedBody ?: messageContent.body)

val bodyToParse = messageContent.formattedBody?.let {
ContentUtils.extractUsefulTextFromHtmlReply(it)
} ?: ContentUtils.extractUsefulTextFromReply(messageContent.body)

val document = parser.parse(bodyToParse)
formattedBody = eventHtmlRenderer.render(document, pillsPostProcessor)
}
views.composerRelatedMessageContent.text = (formattedBody ?: nonFormattedBody)
Expand Down
Loading