From 8dfea1f5a7af77f14a6272e694f576d159e09ffc Mon Sep 17 00:00:00 2001 From: Mirian Margiani Date: Thu, 5 Dec 2024 17:08:00 +0100 Subject: [PATCH 1/3] Pass message permalink to createReplyContent() Signed-off-by: Mirian Margiani --- src/domain/session/room/timeline/tiles/BaseMessageTile.js | 3 ++- src/matrix/room/timeline/entries/BaseEventEntry.js | 5 +++-- src/matrix/room/timeline/entries/reply.js | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/domain/session/room/timeline/tiles/BaseMessageTile.js b/src/domain/session/room/timeline/tiles/BaseMessageTile.js index ed80f4d048..9a5f24f092 100644 --- a/src/domain/session/room/timeline/tiles/BaseMessageTile.js +++ b/src/domain/session/room/timeline/tiles/BaseMessageTile.js @@ -1,5 +1,6 @@ /* Copyright 2020 Bruno Windels +Copyright 2024 Mirian Margiani Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -147,7 +148,7 @@ export class BaseMessageTile extends SimpleTile { } createReplyContent(msgtype, body) { - return this._entry.createReplyContent(msgtype, body); + return this._entry.createReplyContent(msgtype, body, this.permaLink); } redact(reason, log) { diff --git a/src/matrix/room/timeline/entries/BaseEventEntry.js b/src/matrix/room/timeline/entries/BaseEventEntry.js index 32fa807080..154d07dca9 100644 --- a/src/matrix/room/timeline/entries/BaseEventEntry.js +++ b/src/matrix/room/timeline/entries/BaseEventEntry.js @@ -1,5 +1,6 @@ /* Copyright 2021 The Matrix.org Foundation C.I.C. +Copyright 2024 Mirian Margiani Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -185,8 +186,8 @@ export class BaseEventEntry extends BaseEntry { return createAnnotation(this.id, key); } - createReplyContent(msgtype, body) { - return createReplyContent(this, msgtype, body); + createReplyContent(msgtype, body, permaLink) { + return createReplyContent(this, msgtype, body, permaLink); } /** takes both remote event id and local txn id into account, see overriding in PendingEventEntry */ diff --git a/src/matrix/room/timeline/entries/reply.js b/src/matrix/room/timeline/entries/reply.js index 2e180c1124..6039a0e092 100644 --- a/src/matrix/room/timeline/entries/reply.js +++ b/src/matrix/room/timeline/entries/reply.js @@ -1,5 +1,6 @@ /* Copyright 2021 The Matrix.org Foundation C.I.C. +Copyright 2024 Mirian Margiani Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -50,7 +51,7 @@ function _createReplyContent(targetId, msgtype, body, formattedBody) { }; } -export function createReplyContent(entry, msgtype, body) { +export function createReplyContent(entry, msgtype, body, permaLink) { // TODO check for absense of sender / body / msgtype / etc? const nonTextual = fallbackForNonTextualMessage(entry.content.msgtype); const prefix = fallbackPrefix(entry.content.msgtype); From fdb76218770f8180bffdbfbe297ea0f9576a0c16 Mon Sep 17 00:00:00 2001 From: Mirian Margiani Date: Thu, 5 Dec 2024 17:17:19 +0100 Subject: [PATCH 2/3] Refactor reply formatting to follow the 1.12 spec Due to invalid formatting, replies to replies became garbled, causing display issues in some clients. Hydrogen itself managed to display the replies correctly but other clients and bridges struggled because they were actually using the fallbacks. Current spec: https://spec.matrix.org/v1.12/client-server-api/#fallbacks-for-rich-replies Reply fallbacks are actively being removed in the upcoming spec but that doesn't mean that Hydrogen should keep the old bugged code in place. Upcoming MSCs: - https://github.com/matrix-org/matrix-spec-proposals/pull/2781 - https://github.com/matrix-org/matrix-spec-proposals/pull/3676 - spec: https://github.com/matrix-org/matrix-spec/pull/1994 Signed-off-by: Mirian Margiani --- src/matrix/room/timeline/entries/reply.js | 69 +++++++++++++++++++---- 1 file changed, 58 insertions(+), 11 deletions(-) diff --git a/src/matrix/room/timeline/entries/reply.js b/src/matrix/room/timeline/entries/reply.js index 6039a0e092..92b573137b 100644 --- a/src/matrix/room/timeline/entries/reply.js +++ b/src/matrix/room/timeline/entries/reply.js @@ -37,6 +37,41 @@ function fallbackPrefix(msgtype) { return msgtype === "m.emote" ? "* " : ""; } +function _parsePlainBody(plainBody) { + // Strip any existing reply fallback and return an array of lines. + + const bodyLines = plainBody.trim().split("\n"); + + return bodyLines + .map((elem, index, array) => { + if (index > 0 && array[index-1][0] !== '>') { + // stop stripping the fallback at the first line of non-fallback text + return elem; + } else if (elem[0] === '>' && elem[1] === ' ') { + return null; + } else { + return elem; + } + }) + .filter((elem) => elem !== null) + // Join, trim, and split to remove any line breaks that were left between the + // fallback and the actual message body. Don't use trim() because that would + // also remove any other whitespace at the beginning of the message that the + // user added intentionally. + .join('\n') + .replace(/^\n+|\n+$/g, '') + .split('\n') +} + +function _parseFormattedBody(formattedBody) { + // Strip any existing reply fallback and return a HTML string again. + + // This is greedy and definitely not the most efficient way to do it. + // However, this function is only called when sending a reply (so: not too + // often) and it should make sure that all instances of are gone. + return formattedBody.replace(/[\s\S]*<\/mx-reply>/gi, ''); +} + function _createReplyContent(targetId, msgtype, body, formattedBody) { return { msgtype, @@ -48,28 +83,40 @@ function _createReplyContent(targetId, msgtype, body, formattedBody) { "event_id": targetId } } + // TODO include user mentions }; } export function createReplyContent(entry, msgtype, body, permaLink) { - // TODO check for absense of sender / body / msgtype / etc? + // NOTE We assume sender, body, and msgtype are never invalid because they + // are required fields. const nonTextual = fallbackForNonTextualMessage(entry.content.msgtype); const prefix = fallbackPrefix(entry.content.msgtype); const sender = entry.sender; - const name = entry.displayName || sender; - - const formattedBody = nonTextual || entry.content.formatted_body || - (entry.content.body && htmlEscape(entry.content.body)) || ""; - const formattedFallback = `
In reply to ${prefix}` + - `${name}
` + - `${formattedBody}
`; + const repliedToId = entry.id; + // TODO collect user mentions (sender and any previous mentions) + // Generate new plain body with plain reply fallback const plainBody = nonTextual || entry.content.body || ""; - const bodyLines = plainBody.split("\n"); + const bodyLines = _parsePlainBody(plainBody); bodyLines[0] = `> ${prefix}<${sender}> ${bodyLines[0]}` const plainFallback = bodyLines.join("\n> "); - const newBody = plainFallback + '\n\n' + body; - const newFormattedBody = formattedFallback + htmlEscape(body); + + // Generate new formatted body with formatted reply fallback + const formattedBody = nonTextual || entry.content.formatted_body || + (entry.content.body && htmlEscape(entry.content.body)) || ""; + const cleanedFormattedBody = _parseFormattedBody(formattedBody); + const formattedFallback = + `` + + `
` + + `In reply to` + + `${prefix}${sender}` + + `
` + + `${cleanedFormattedBody}` + + `
` + + ``; + const newFormattedBody = formattedFallback + htmlEscape(body).replaceAll('\n', '
'); + return _createReplyContent(entry.id, msgtype, newBody, newFormattedBody); } From 4dd459da786302e473c6c2864c5fd0577d5fc31a Mon Sep 17 00:00:00 2001 From: Mirian Margiani Date: Thu, 5 Dec 2024 17:30:36 +0100 Subject: [PATCH 3/3] Add m.mentions for replied-to messages Mention the replied-to user when replying to a message as per https://spec.matrix.org/v1.12/client-server-api/#mentioning-the-replied-to-user Signed-off-by: Mirian Margiani --- src/matrix/room/timeline/entries/reply.js | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/matrix/room/timeline/entries/reply.js b/src/matrix/room/timeline/entries/reply.js index 92b573137b..23e464ee37 100644 --- a/src/matrix/room/timeline/entries/reply.js +++ b/src/matrix/room/timeline/entries/reply.js @@ -72,7 +72,7 @@ function _parseFormattedBody(formattedBody) { return formattedBody.replace(/[\s\S]*<\/mx-reply>/gi, ''); } -function _createReplyContent(targetId, msgtype, body, formattedBody) { +function _createReplyContent(targetId, targetSenderId, msgtype, body, formattedBody) { return { msgtype, body, @@ -82,8 +82,12 @@ function _createReplyContent(targetId, msgtype, body, formattedBody) { "m.in_reply_to": { "event_id": targetId } + }, + "m.mentions": { + "user_ids": [ + targetSenderId, + ] } - // TODO include user mentions }; } @@ -94,7 +98,16 @@ export function createReplyContent(entry, msgtype, body, permaLink) { const prefix = fallbackPrefix(entry.content.msgtype); const sender = entry.sender; const repliedToId = entry.id; + // TODO collect user mentions (sender and any previous mentions) + // Considerations: + // - Who should be included in the mentions? In a reply chain, should all + // previous mentions be carried over indefinitely? How to decide when to + // stop carrying mentions? + // - Don't add a mentions section when replying to own messages without + // any other mentions. As per https://spec.matrix.org/v1.12/client-server-api/#user-and-room-mentions + // "Users should not add their own Matrix ID to the m.mentions property + // as outgoing messages cannot self-notify." // Generate new plain body with plain reply fallback const plainBody = nonTextual || entry.content.body || ""; @@ -118,5 +131,5 @@ export function createReplyContent(entry, msgtype, body, permaLink) { ``; const newFormattedBody = formattedFallback + htmlEscape(body).replaceAll('\n', '
'); - return _createReplyContent(entry.id, msgtype, newBody, newFormattedBody); + return _createReplyContent(repliedToId, sender, msgtype, newBody, newFormattedBody); }