From 8b95ffc77261c2cdc87f5a9f4e7a1d99e62bde1b Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 7 Jul 2020 15:46:55 -0400 Subject: [PATCH 01/28] initial version of message editing proposal --- proposals/xxxx-message-editing.md | 137 ++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 proposals/xxxx-message-editing.md diff --git a/proposals/xxxx-message-editing.md b/proposals/xxxx-message-editing.md new file mode 100644 index 0000000000..6340b06ec2 --- /dev/null +++ b/proposals/xxxx-message-editing.md @@ -0,0 +1,137 @@ +# MSCxxxx: Message editing + +Users may wish to edit previously sent messages, for example to correct typos. +This can be done by sending a new message with an indication that it replaces +the previously sent message. + +This proposal is one in a series of proposals that defines a mechanism for +events to relate to each other. Together, these proposals replace +[MSC1849](https://github.com/matrix-org/matrix-doc/pull/1849). + +* [MSCxxxx](https://github.com/matrix-org/matrix-doc/pull/xxxx) defines a + standard shape for indicating events which relate to other events. +* [MSCxxxx](https://github.com/matrix-org/matrix-doc/pull/xxxx) defines APIs to + let the server calculate the aggregations on behalf of the client, and so + bundle the related events with the original event where appropriate. +* This proposal defines how users can edit messages using this mechanism. +* [MSCxxxx](https://github.com/matrix-org/matrix-doc/pull/xxxx) defines how + users can annotate events, such as reacting to events with emoji, using this + mechanism. + +## Proposal + +A new `rel_type` of `m.replace` is defined for use with the `m.relates_to` +field as defined in +[MSCxxxx](https://github.com/matrix-org/matrix-doc/pull/xxxx). This is +intended primarily for handling edits, and lets you define an event which +replaces an existing event. When aggregated (as in +[MSCxxxx](https://github.com/matrix-org/matrix-doc/pull/xxxx)), returns the +most recent replacement event (as determined by `origin_server_ts`). The +replacement event must contain an `m.new_content` which defines the replacement +content (allowing the normal `body` fields to be used for a fallback for +clients who do not understand replacement events). + +For instance, an `m.room.message` which replaces an existing event looks like: + +```json +{ + "type": "m.room.message", + "content": { + "body": "s/foo/bar/", + "msgtype": "m.text", + "m.new_content": { + "body": "Hello! My name is bar", + "msgtype": "m.text", + }, + "m.relates_to": { + "rel_type": "m.replace", + "event_id": "$some_event_id", + } + } +} +``` + +Permalinks to edited events should capture the event ID that the sender is +viewing at that point (which might be an edit ID). The client viewing the +permalink should resolve this ID to the source event ID, and then display the +most recent version of that event. + +### Redactions + +When a message using a `rel_type` of `m.replace` is redacted, it removes that +edit revision. + +In the UI, the act of redacting an edited message in the timeline should +remove the message entirely from the timeline. It can do this by redacting the +original msg, while ensuring that clients locally discard any edits to a +redacted message on receiving a redaction. + +When a specific revision of an event is redacted, the client should manually +refresh the parent event via `/events` to grab whatever the replacement +revision is. + +## Edge Cases + +How do you handle racing edits? + * The edits could form a DAG of relations for robustness. + * Tie-break between forward DAG extremities based on origin_ts + * this should be different from the target event_id in the relations, to + make it easier to know what is being replaced. + * hard to see who is responsible for linearising the DAG when receiving. + Nasty for the client to do it, but the server would have to buffer, + meaning relations could get stuck if an event in the DAG is unavailable. + * ...or do we just always order by on origin_ts, and rely on a social problem + for it not to be abused? + * problem is that other relation types might well need a more robust way of + ordering. XXX: can we think of any? + * could add the DAG in later if it's really needed? + * the abuse vector is for a malicious moderator to edit a message with origin_ts + of MAX_INT. the mitigation is to redact such malicious messages, although this + does mean the original message ends up being vandalised... :/ + * Conclusion: let's do it for origin_ts as a first cut, but use event shapes which + could be switched to DAG in future is/as needed. Good news is that it only + affects the server implementation; the clients can trust the server to linearise + correctly. + +What can we edit? + * Only non-state events for now. + * We can't change event types, or anything else which is in an E2E payload + * We can't change relation types either. + +How do diffs work on edits if you are missing intermediary edits? + * We just have to ensure that the UI for visualising diffs makes it clear + that diffs could span multiple edits rather than strictly be per-edit-event. + +What happens when we edit a reply? + * We just send an m.replace which refers to the m.reference target; nothing + special is needed. i.e. you cannot change who the event is replying to. + * The edited reply should ditch the fallback representation of the reply itself + however from `m.new_content` (specifically the `` tag in the + HTML, and the chevron prefixed text in the plaintext which we don't know + whether to parse as we don't know whether this is a reply or not), as we + can assume that any client which can handle edits can also display replies + natively. + + XXX: make Riot do this + +What power level do you need to be able to edit other people's messages, and how +does it fit in with fedeation event auth rules? + * 50, by default? + + XXX: Synapse doesn't impose this currently - it lets anyone send an edit, + but then filters them out of bundled data. + +"Editing other people's messages is evil; we shouldn't allow it" + * Sorry, we have to bridge with systems which support cross-user edits. + * When it happens, we should make it super clear in the timeline that a message + was edited by a specific user. + * We do not recommend that native Matrix clients expose this as a feature. + +## Future considerations + +In future we may wish to consider ordering replacements (or relations in +general) via a DAG rather than using `origin_server_ts` to determine ordering - +particularly to mitigate potential abuse of edits applied by moderators. +Whatever, care must be taken by the server to ensure that if there are multiple +replacement events, the server must consistently choose the same one as all +other servers. From 7f42c8b7e5bfb7f54ba1770dce7bf66372af10e5 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 7 Jul 2020 16:26:58 -0400 Subject: [PATCH 02/28] fix MSC numbers --- ...xx-message-editing.md => 2676-message-editing.md} | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) rename proposals/{xxxx-message-editing.md => 2676-message-editing.md} (94%) diff --git a/proposals/xxxx-message-editing.md b/proposals/2676-message-editing.md similarity index 94% rename from proposals/xxxx-message-editing.md rename to proposals/2676-message-editing.md index 6340b06ec2..b824cf5b7c 100644 --- a/proposals/xxxx-message-editing.md +++ b/proposals/2676-message-editing.md @@ -1,4 +1,4 @@ -# MSCxxxx: Message editing +# MSC2676: Message editing Users may wish to edit previously sent messages, for example to correct typos. This can be done by sending a new message with an indication that it replaces @@ -8,13 +8,13 @@ This proposal is one in a series of proposals that defines a mechanism for events to relate to each other. Together, these proposals replace [MSC1849](https://github.com/matrix-org/matrix-doc/pull/1849). -* [MSCxxxx](https://github.com/matrix-org/matrix-doc/pull/xxxx) defines a +* [MSC2674](https://github.com/matrix-org/matrix-doc/pull/2674) defines a standard shape for indicating events which relate to other events. -* [MSCxxxx](https://github.com/matrix-org/matrix-doc/pull/xxxx) defines APIs to +* [MSC2675](https://github.com/matrix-org/matrix-doc/pull/2675) defines APIs to let the server calculate the aggregations on behalf of the client, and so bundle the related events with the original event where appropriate. * This proposal defines how users can edit messages using this mechanism. -* [MSCxxxx](https://github.com/matrix-org/matrix-doc/pull/xxxx) defines how +* [MSC2677](https://github.com/matrix-org/matrix-doc/pull/2677) defines how users can annotate events, such as reacting to events with emoji, using this mechanism. @@ -22,10 +22,10 @@ events to relate to each other. Together, these proposals replace A new `rel_type` of `m.replace` is defined for use with the `m.relates_to` field as defined in -[MSCxxxx](https://github.com/matrix-org/matrix-doc/pull/xxxx). This is +[MSC2674](https://github.com/matrix-org/matrix-doc/pull/2674). This is intended primarily for handling edits, and lets you define an event which replaces an existing event. When aggregated (as in -[MSCxxxx](https://github.com/matrix-org/matrix-doc/pull/xxxx)), returns the +[MSC2675](https://github.com/matrix-org/matrix-doc/pull/2675)), returns the most recent replacement event (as determined by `origin_server_ts`). The replacement event must contain an `m.new_content` which defines the replacement content (allowing the normal `body` fields to be used for a fallback for From 547300964b7ada14048b3bcbcbf2c9e6b708bf8d Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 24 Nov 2020 09:46:36 -0500 Subject: [PATCH 03/28] Fix JSON in example Co-authored-by: Alexandre Morignot --- proposals/2676-message-editing.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index b824cf5b7c..2e933e24dd 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -41,11 +41,11 @@ For instance, an `m.room.message` which replaces an existing event looks like: "msgtype": "m.text", "m.new_content": { "body": "Hello! My name is bar", - "msgtype": "m.text", + "msgtype": "m.text" }, "m.relates_to": { "rel_type": "m.replace", - "event_id": "$some_event_id", + "event_id": "$some_event_id" } } } From e44f56666bda96df2b0d4590f707101ec6155b34 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 2 Jun 2021 17:32:48 -0400 Subject: [PATCH 04/28] clarifications --- proposals/2676-message-editing.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index 2e933e24dd..50840d1e53 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -51,6 +51,13 @@ For instance, an `m.room.message` which replaces an existing event looks like: } ``` +The `m.new_content` includes any fields that would normally be found in an +event's `content` field, such as `formatted_body`. In addition, the `msgtype` +field need not be the same as in the original event. For example, if a user +intended to send a message beginning with "/me", but their client sends an +`m.emote` event instead, they could edit the message to send be an `m.text` +event as they had originally intended. + Permalinks to edited events should capture the event ID that the sender is viewing at that point (which might be an edit ID). The client viewing the permalink should resolve this ID to the source event ID, and then display the @@ -112,7 +119,7 @@ What happens when we edit a reply? can assume that any client which can handle edits can also display replies natively. - XXX: make Riot do this + XXX: make Element do this What power level do you need to be able to edit other people's messages, and how does it fit in with fedeation event auth rules? From 634dc2e89f7db0680d7244472ab17e7f8fb5a472 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Thu, 28 Oct 2021 09:26:12 -0400 Subject: [PATCH 05/28] remove obsolete "XXX:", and fix a typo --- proposals/2676-message-editing.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index 50840d1e53..a181b30c72 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -119,10 +119,8 @@ What happens when we edit a reply? can assume that any client which can handle edits can also display replies natively. - XXX: make Element do this - What power level do you need to be able to edit other people's messages, and how -does it fit in with fedeation event auth rules? +does it fit in with federation event auth rules? * 50, by default? XXX: Synapse doesn't impose this currently - it lets anyone send an edit, From f9768e7d6c6bf105371e8000fa298a168cfddc03 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 12 Apr 2022 15:25:13 +0100 Subject: [PATCH 06/28] Initial cleanup and restructuring --- proposals/2676-message-editing.md | 55 ++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index a181b30c72..04e18d7a74 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -20,24 +20,25 @@ events to relate to each other. Together, these proposals replace ## Proposal +### `m.replace` event relationship type + A new `rel_type` of `m.replace` is defined for use with the `m.relates_to` field as defined in -[MSC2674](https://github.com/matrix-org/matrix-doc/pull/2674). This is +[MSC2674](https://github.com/matrix-org/matrix-doc/pull/2674). This is intended primarily for handling edits, and lets you define an event which -replaces an existing event. When aggregated (as in -[MSC2675](https://github.com/matrix-org/matrix-doc/pull/2675)), returns the -most recent replacement event (as determined by `origin_server_ts`). The -replacement event must contain an `m.new_content` which defines the replacement -content (allowing the normal `body` fields to be used for a fallback for -clients who do not understand replacement events). +replaces an existing event. + +The replacement event must contain a `m.new_content` property which defines the +replacement content. (This allows the normal `body` fields to be used for a +fallback for clients who do not understand replacement events.) -For instance, an `m.room.message` which replaces an existing event looks like: +For instance, an `m.room.message` which replaces an existing event might look like: ```json { "type": "m.room.message", "content": { - "body": "s/foo/bar/", + "body": "* Hello! My name is bar", "msgtype": "m.text", "m.new_content": { "body": "Hello! My name is bar", @@ -51,9 +52,9 @@ For instance, an `m.room.message` which replaces an existing event looks like: } ``` -The `m.new_content` includes any fields that would normally be found in an +The `m.new_content` can include any fields that would normally be found in an event's `content` field, such as `formatted_body`. In addition, the `msgtype` -field need not be the same as in the original event. For example, if a user +field need not be the same as in the original event. For example, if a user intended to send a message beginning with "/me", but their client sends an `m.emote` event instead, they could edit the message to send be an `m.text` event as they had originally intended. @@ -77,6 +78,35 @@ When a specific revision of an event is redacted, the client should manually refresh the parent event via `/events` to grab whatever the replacement revision is. +### Server-side aggregation of `m.replace` relationships + +Note that there can be multiple event with an `m.replace` relationship to a +given event (for example, if an event is edited multiple times). Homeservers +should aggregate `m.replace` relationships as in +[MSC2675](https://github.com/matrix-org/matrix-doc/pull/2675). The aggregation +gives the `event_id`, `origin_server_ts`, and `sender` of the most recent +replacement event (as determined by `origin_server_ts`). + +This aggregation is bundled into the `unsigned/m.relations` property of any +event that is the target of an `m.replace` relationship. For example: + +```json5 + +{ + "event_id": "$original_event_id", + // ... + "unsigned": { + "m.relations": { + "m.replace": { + "event_id": "$latest_edit_event_id", + "origin_server_ts": 1649772304313, + "sender": "@editing_user:localhost + } + } + } +} +``` + ## Edge Cases How do you handle racing edits? @@ -132,6 +162,9 @@ does it fit in with federation event auth rules? was edited by a specific user. * We do not recommend that native Matrix clients expose this as a feature. + +what happens if `m.new_content` is absent? or the `type` is different? + ## Future considerations In future we may wish to consider ordering replacements (or relations in From 1de6c11026b7eb1e48f606a4067fea4ec8d52495 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 12 Apr 2022 16:18:02 +0100 Subject: [PATCH 07/28] Clarify algorithm for replacing content --- proposals/2676-message-editing.md | 71 ++++++++++++++++++++++++++++--- 1 file changed, 65 insertions(+), 6 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index 04e18d7a74..c77275fcd2 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -52,12 +52,71 @@ For instance, an `m.room.message` which replaces an existing event might look li } ``` -The `m.new_content` can include any fields that would normally be found in an -event's `content` field, such as `formatted_body`. In addition, the `msgtype` -field need not be the same as in the original event. For example, if a user -intended to send a message beginning with "/me", but their client sends an -`m.emote` event instead, they could edit the message to send be an `m.text` -event as they had originally intended. +The `m.new_content` can include any properties that would normally be found in +an event's `content` property, such as `formatted_body`. + +Note that the `msgtype` property of `m.room.message` events need not be the +same as in the original event. For example, if a user intended to send a +message beginning with "/me", but their client sends an `m.emote` event +instead, they could edit the message to send be an `m.text` event as they had +originally intended. + +#### Applying `m.new_content` + +When updating an existing event for an `m.replace` relating event, the old +`content` property is replaced entirely by the `m.new_content`, with the +exception of `m.relates_to`, which is left *unchanged*. For example, given a +pair of events: + +```json +{ + "event_id": "$original_event", + "type": "m.room.message", + "content": { + "body": "I *really* like cake", + "msgtype": "m.text", + "formatted_body": "I really like cake", + } +} +``` + +```json +{ + "event_id": "$edit_event", + "type": "m.room.message", + "content": { + "body": "* I *really* like *chocolate* cake", + "msgtype": "m.text", + "m.new_content": { + "body": "I *really* like *chocolate* cake", + "msgtype": "m.text", + "com.example.extension_property": "chocolate" + }, + "m.relates_to": { + "rel_type": "m.replace", + "event_id": "$original_event_id" + } + } +} +``` + +... then the end result is an event as shown below. Note that `formatted_body` +is now absent, because it was absent in the replacement event, but +`m.relates_to` remains unchanged (ie, absent). + +```json +{ + "event_id": "$original_event", + "type": "m.room.message", + "content": { + "body": "I *really* like *chocolate* cake", + "msgtype": "m.text", + "com.example.extension_property": "chocolate" + } +} +``` + +### Permalinks Permalinks to edited events should capture the event ID that the sender is viewing at that point (which might be an edit ID). The client viewing the From adcdddc163cc1b908af686b0c40fd60c604eeb9e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 12 Apr 2022 18:14:48 +0100 Subject: [PATCH 08/28] background --- proposals/2676-message-editing.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index c77275fcd2..50eee620cf 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -18,6 +18,32 @@ events to relate to each other. Together, these proposals replace users can annotate events, such as reacting to events with emoji, using this mechanism. +## Background + +Element-Web (then Riot-Web) and Synapse both implemented initial support for +message editing, following the proposals of MSC1849, in May 2019 +([matrix-react-sdk](https://github.com/matrix-org/matrix-react-sdk/pull/2952), +[synapse](https://github.com/matrix-org/synapse/pull/5209)). Element-Android +and Element-iOS also added implementations around that time. Unfortunately, +those implementations presented the feature as "production-ready", despite it +not yet having been adopted into the Matrix specification. + +The current situation is therefore that client or server implementations hoping +to interact with Element users must simply follow the examples of that +implementation. In other words, message edits form part of the *de-facto* spec +despite not being formalised in the written spec. This is clearly a regrettable +situation. Hopefully, processes have improved over the last three years so that +this situation will not arise again. Nevertheless there is little we can do +now other than formalise the status quo. + +This MSC, along with the others mentioned above, therefore seeks primarily to +do that. Although there is plenty of scope for improvement, we consider that +better done in *future* MSCs, based on a shared understaning of the *current* +implementation. + +In short, this MSC prefers fidelity to the current implementations over +elegance of design. + ## Proposal ### `m.replace` event relationship type From fc2a6908aec71abbe833379d12845038af321a90 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 12 Apr 2022 18:54:54 +0100 Subject: [PATCH 09/28] More clarifications on applying edits --- proposals/2676-message-editing.md | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index 50eee620cf..45960efc41 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -78,6 +78,8 @@ For instance, an `m.room.message` which replaces an existing event might look li } ``` +Such an event, with `rel_type: m.replace`, is referred to as a "message edit". + The `m.new_content` can include any properties that would normally be found in an event's `content` property, such as `formatted_body`. @@ -87,12 +89,28 @@ message beginning with "/me", but their client sends an `m.emote` event instead, they could edit the message to send be an `m.text` event as they had originally intended. +Whenever a homeserver would return an event via the Client-Server API, it +should check for any applicable `m.replace` event, and if one is found, it +should first modify the `content` of the original event according to the +`m.new_content` of the most recent edit (as determined by +`origin_server_ts`). An exception applies to [`GET +/_matrix/client/v3/rooms/{roomId}/event/{eventId}`](https://spec.matrix.org/v1.2/client-server-api/#get_matrixclientv3roomsroomideventeventid), +which should return the *unmodified* event (though the relationship should still be +"bundled", as described [below](#server-side-aggregation-of-mreplace-relationships). + +Clients are generally expected to ignore message edit events, since the server +implementation takes care of updating the content of the original +event. However, if the client has already received the original event, it must +apply the replacement to the original itself (or, alternatively, request an +updated copy of the original via [`GET +/_matrix/client/v3/rooms/{roomId}/context/{eventId}`](https://spec.matrix.org/v1.2/client-server-api/#get_matrixclientv3roomsroomidcontexteventid) +or similar). + #### Applying `m.new_content` -When updating an existing event for an `m.replace` relating event, the old -`content` property is replaced entirely by the `m.new_content`, with the -exception of `m.relates_to`, which is left *unchanged*. For example, given a -pair of events: +When applying a replacement, the `content` property of the origial event is +replaced entirely by the `m.new_content`, with the exception of `m.relates_to`, +which is left *unchanged*. For example, given a pair of events: ```json { From 80c467b4e9ae389ea5d74e38d9028ad174dea4ba Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 12 Apr 2022 19:31:30 +0100 Subject: [PATCH 10/28] Clarify behaviour of redactions --- proposals/2676-message-editing.md | 34 +++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index 45960efc41..bd5fd9f280 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -170,16 +170,16 @@ most recent version of that event. ### Redactions When a message using a `rel_type` of `m.replace` is redacted, it removes that -edit revision. +edit revision. This has little effect if there were subsequent edits, however +if it was the most recent edit, the event is in effect reverted to its content +before the redacted edit. -In the UI, the act of redacting an edited message in the timeline should -remove the message entirely from the timeline. It can do this by redacting the -original msg, while ensuring that clients locally discard any edits to a -redacted message on receiving a redaction. - -When a specific revision of an event is redacted, the client should manually -refresh the parent event via `/events` to grab whatever the replacement -revision is. +Redacting the original message in effect removes the message, including all +subsequent edits, from the visible timeline. In this situation, homeservers +will return an empty `content` for the original event as with any other +redacted event. It must be noted that, although they are not immediately +visible in Element, subsequent edits remain unredacted and can be seen via API +calls. See [Future considerations](#future-considerations). ### Server-side aggregation of `m.replace` relationships @@ -210,6 +210,11 @@ event that is the target of an `m.replace` relationship. For example: } ``` +If the original event is redacted, any `m.replace` relationship should **not** +be bundled with it (whether or not any subsequent edits are themselves +redacted). Note that this behaviour is specific to the `m.replace` +relationship. + ## Edge Cases How do you handle racing edits? @@ -270,9 +275,20 @@ what happens if `m.new_content` is absent? or the `type` is different? ## Future considerations +### Ordering of edits + In future we may wish to consider ordering replacements (or relations in general) via a DAG rather than using `origin_server_ts` to determine ordering - particularly to mitigate potential abuse of edits applied by moderators. Whatever, care must be taken by the server to ensure that if there are multiple replacement events, the server must consistently choose the same one as all other servers. + +### Redaction of edits + +It is highly unintuitive that redacting the original event leaves subsequent +edits visible to curious eyes even though they are hidden from the +timeline. This is considered a bug which this MSC makes no attempt to +resolve. See also +[element-web#11978](https://github.com/vector-im/element-web/issues/11978) and +[synapse#5594](https://github.com/matrix-org/synapse/issues/5594). From 6cea76a54a9b673374b7c24639702a42344f98d6 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 17 May 2022 16:16:23 +0100 Subject: [PATCH 11/28] Minor grammar fixes --- proposals/2676-message-editing.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index bd5fd9f280..b0e4a40ab0 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -108,7 +108,7 @@ or similar). #### Applying `m.new_content` -When applying a replacement, the `content` property of the origial event is +When applying a replacement, the `content` property of the original event is replaced entirely by the `m.new_content`, with the exception of `m.relates_to`, which is left *unchanged*. For example, given a pair of events: @@ -183,7 +183,7 @@ calls. See [Future considerations](#future-considerations). ### Server-side aggregation of `m.replace` relationships -Note that there can be multiple event with an `m.replace` relationship to a +Note that there can be multiple events with an `m.replace` relationship to a given event (for example, if an event is edited multiple times). Homeservers should aggregate `m.replace` relationships as in [MSC2675](https://github.com/matrix-org/matrix-doc/pull/2675). The aggregation From dac2399029d37eead70c94e162e780c9aee831c3 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 17 May 2022 16:16:57 +0100 Subject: [PATCH 12/28] Move the section on `msgtype` down It clutters up the initial description - let's move it down into a more detailed section. --- proposals/2676-message-editing.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index b0e4a40ab0..6ce647750e 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -83,12 +83,6 @@ Such an event, with `rel_type: m.replace`, is referred to as a "message edit". The `m.new_content` can include any properties that would normally be found in an event's `content` property, such as `formatted_body`. -Note that the `msgtype` property of `m.room.message` events need not be the -same as in the original event. For example, if a user intended to send a -message beginning with "/me", but their client sends an `m.emote` event -instead, they could edit the message to send be an `m.text` event as they had -originally intended. - Whenever a homeserver would return an event via the Client-Server API, it should check for any applicable `m.replace` event, and if one is found, it should first modify the `content` of the original event according to the @@ -160,6 +154,13 @@ is now absent, because it was absent in the replacement event, but } ``` +Note that the `msgtype` property of `m.room.message` events need not be the +same as in the original event. For example, if a user intended to send a +message beginning with "/me", but their client sends an `m.emote` event +instead, they could edit the message to send be an `m.text` event as they had +originally intended. + + ### Permalinks Permalinks to edited events should capture the event ID that the sender is From b8a7745b782733178a11c2329f36a7cf17e49a81 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 17 May 2022 16:19:21 +0100 Subject: [PATCH 13/28] Clarify how edits are ordered - we use event_id as a tiebreaker. We also have a section in "Future considerations" about this, so I don't think we need the braindump in "Edge cases". --- proposals/2676-message-editing.md | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index 6ce647750e..a6500d0a8f 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -87,7 +87,7 @@ Whenever a homeserver would return an event via the Client-Server API, it should check for any applicable `m.replace` event, and if one is found, it should first modify the `content` of the original event according to the `m.new_content` of the most recent edit (as determined by -`origin_server_ts`). An exception applies to [`GET +`origin_server_ts`, falling back to `event_id`). An exception applies to [`GET /_matrix/client/v3/rooms/{roomId}/event/{eventId}`](https://spec.matrix.org/v1.2/client-server-api/#get_matrixclientv3roomsroomideventeventid), which should return the *unmodified* event (though the relationship should still be "bundled", as described [below](#server-side-aggregation-of-mreplace-relationships). @@ -218,27 +218,6 @@ relationship. ## Edge Cases -How do you handle racing edits? - * The edits could form a DAG of relations for robustness. - * Tie-break between forward DAG extremities based on origin_ts - * this should be different from the target event_id in the relations, to - make it easier to know what is being replaced. - * hard to see who is responsible for linearising the DAG when receiving. - Nasty for the client to do it, but the server would have to buffer, - meaning relations could get stuck if an event in the DAG is unavailable. - * ...or do we just always order by on origin_ts, and rely on a social problem - for it not to be abused? - * problem is that other relation types might well need a more robust way of - ordering. XXX: can we think of any? - * could add the DAG in later if it's really needed? - * the abuse vector is for a malicious moderator to edit a message with origin_ts - of MAX_INT. the mitigation is to redact such malicious messages, although this - does mean the original message ends up being vandalised... :/ - * Conclusion: let's do it for origin_ts as a first cut, but use event shapes which - could be switched to DAG in future is/as needed. Good news is that it only - affects the server implementation; the clients can trust the server to linearise - correctly. - What can we edit? * Only non-state events for now. * We can't change event types, or anything else which is in an E2E payload From 79a362e47f2d88339371d2c7df73a6c6ed7a909c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 17 May 2022 17:14:03 +0100 Subject: [PATCH 14/28] Spec the behaviour for encrypted events --- proposals/2676-message-editing.md | 61 ++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 5 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index a6500d0a8f..45cbe2010c 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -54,9 +54,13 @@ field as defined in intended primarily for handling edits, and lets you define an event which replaces an existing event. -The replacement event must contain a `m.new_content` property which defines the -replacement content. (This allows the normal `body` fields to be used for a -fallback for clients who do not understand replacement events.) +Such an event, with `rel_type: m.replace`, is referred to as a "message edit event". + +### `m.new_content` property + +The `content` of a message edit event must contain a `m.new_content` property +which defines the replacement content. (This allows the normal `body` fields to +be used for a fallback for clients who do not understand replacement events.) For instance, an `m.room.message` which replaces an existing event might look like: @@ -78,11 +82,56 @@ For instance, an `m.room.message` which replaces an existing event might look li } ``` -Such an event, with `rel_type: m.replace`, is referred to as a "message edit". - The `m.new_content` can include any properties that would normally be found in an event's `content` property, such as `formatted_body`. +#### Encrypted events + +If the original event was encrypted, the replacement should be too. In that +case, `m.new_content` is placed in the `content` of the encrypted payload. For +example, an encrypted replacement event might look like this: + +```json +{ + "type": "m.room.encrypted", + "content": { + "m.relates_to": { + "rel_type": "m.replace", + "event_id": "$some_event_id" + }, + "algorithm": "m.megolm.v1.aes-sha2", + "sender_key": "", + "device_id": "", + "session_id": "", + "ciphertext": "" + } +} +``` + +... and, once decrypted, the payload might look like this: + + +```json +{ + "type": "m.room.", + "room_id": "!some_room_id", + "content": { + "body": "* Hello! My name is bar", + "msgtype": "m.text", + "m.new_content": { + "body": "Hello! My name is bar", + "msgtype": "m.text" + }, + "m.relates_to": { + "rel_type": "m.replace", + "event_id": "$some_event_id" + } + } +} +``` + +#### Server behaviour + Whenever a homeserver would return an event via the Client-Server API, it should check for any applicable `m.replace` event, and if one is found, it should first modify the `content` of the original event according to the @@ -92,6 +141,8 @@ should first modify the `content` of the original event according to the which should return the *unmodified* event (though the relationship should still be "bundled", as described [below](#server-side-aggregation-of-mreplace-relationships). +#### Client behaviour + Clients are generally expected to ignore message edit events, since the server implementation takes care of updating the content of the original event. However, if the client has already received the original event, it must From bb96694ca7e0ec7d04320759309bd1462eacd2dc Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 17 May 2022 17:41:34 +0100 Subject: [PATCH 15/28] Requirements for an edit event to be considered valid --- proposals/2676-message-editing.md | 54 +++++++++++++++++++------------ 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index 45cbe2010c..1bd3192d01 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -211,6 +211,23 @@ message beginning with "/me", but their client sends an `m.emote` event instead, they could edit the message to send be an `m.text` event as they had originally intended. +### Validity of message edit events + +Some message edit events are defined to be invalid. To be considered valid, all +of the following criteria must be satisfied: + + * The replacement and original events must have the same `type`. + * Neither the replacement nor original events can be state events (ie, neither + may have a `state_key`). + * The original event must not, itself, have a `rel_type` of `m.replace`. + * The original event and replacement event must have the same `room_id`. + * The original event and replacement event must have the same `sender`. + * The replacement event (once decrypted, if appropriate) must have an + `m.new_content` property. + +If any of these criteria are not satisfied, implementations should ignore the +replacement event (the content of the original should not be replaced, and the +edit should not be included in the server-side aggregation). ### Permalinks @@ -267,12 +284,8 @@ be bundled with it (whether or not any subsequent edits are themselves redacted). Note that this behaviour is specific to the `m.replace` relationship. -## Edge Cases -What can we edit? - * Only non-state events for now. - * We can't change event types, or anything else which is in an E2E payload - * We can't change relation types either. +## Edge Cases How do diffs work on edits if you are missing intermediary edits? * We just have to ensure that the UI for visualising diffs makes it clear @@ -288,21 +301,6 @@ What happens when we edit a reply? can assume that any client which can handle edits can also display replies natively. -What power level do you need to be able to edit other people's messages, and how -does it fit in with federation event auth rules? - * 50, by default? - - XXX: Synapse doesn't impose this currently - it lets anyone send an edit, - but then filters them out of bundled data. - -"Editing other people's messages is evil; we shouldn't allow it" - * Sorry, we have to bridge with systems which support cross-user edits. - * When it happens, we should make it super clear in the timeline that a message - was edited by a specific user. - * We do not recommend that native Matrix clients expose this as a feature. - - -what happens if `m.new_content` is absent? or the `type` is different? ## Future considerations @@ -323,3 +321,19 @@ timeline. This is considered a bug which this MSC makes no attempt to resolve. See also [element-web#11978](https://github.com/vector-im/element-web/issues/11978) and [synapse#5594](https://github.com/matrix-org/synapse/issues/5594). + +### Edits to state events + +There are various issues which would need to be resolved before edits to state +events could be supported. In particular, we would need to consider how the +semantically-meaningful fields of the content of a state event relate to +`m.new_content`. Variation between implementations could easily lead to +security problems (See +[element-web#21851](https://github.com/vector-im/element-web/issues/21851) for +example.) + +### Editing other users' events + +There is a usecase for users with sufficient power-level to edit other peoples' +events. For now, no attempt is made to support this. If it is supported in the +future, we would need to find a way to make it clear in the timeline. From dd1ca71977d92f181db4021926e64e545b0a6f41 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 17 May 2022 17:59:08 +0100 Subject: [PATCH 16/28] Collect "client behaviour" and "sever behaviour" together ... and clarify these sections. --- proposals/2676-message-editing.md | 94 ++++++++++++++++++------------- 1 file changed, 54 insertions(+), 40 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index 1bd3192d01..ca5ae46c59 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -130,27 +130,6 @@ example, an encrypted replacement event might look like this: } ``` -#### Server behaviour - -Whenever a homeserver would return an event via the Client-Server API, it -should check for any applicable `m.replace` event, and if one is found, it -should first modify the `content` of the original event according to the -`m.new_content` of the most recent edit (as determined by -`origin_server_ts`, falling back to `event_id`). An exception applies to [`GET -/_matrix/client/v3/rooms/{roomId}/event/{eventId}`](https://spec.matrix.org/v1.2/client-server-api/#get_matrixclientv3roomsroomideventeventid), -which should return the *unmodified* event (though the relationship should still be -"bundled", as described [below](#server-side-aggregation-of-mreplace-relationships). - -#### Client behaviour - -Clients are generally expected to ignore message edit events, since the server -implementation takes care of updating the content of the original -event. However, if the client has already received the original event, it must -apply the replacement to the original itself (or, alternatively, request an -updated copy of the original via [`GET -/_matrix/client/v3/rooms/{roomId}/context/{eventId}`](https://spec.matrix.org/v1.2/client-server-api/#get_matrixclientv3roomsroomidcontexteventid) -or similar). - #### Applying `m.new_content` When applying a replacement, the `content` property of the original event is @@ -229,28 +208,21 @@ If any of these criteria are not satisfied, implementations should ignore the replacement event (the content of the original should not be replaced, and the edit should not be included in the server-side aggregation). -### Permalinks - -Permalinks to edited events should capture the event ID that the sender is -viewing at that point (which might be an edit ID). The client viewing the -permalink should resolve this ID to the source event ID, and then display the -most recent version of that event. - -### Redactions +### Server behaviour -When a message using a `rel_type` of `m.replace` is redacted, it removes that -edit revision. This has little effect if there were subsequent edits, however -if it was the most recent edit, the event is in effect reverted to its content -before the redacted edit. +Whenever a homeserver would return an event via the Client-Server API, it +should check for any valid, applicable `m.replace` event, and if one is found, it +should first modify the `content` of the original event according to the +`m.new_content` of the most recent edit (as determined by +`origin_server_ts`, falling back to `event_id`). -Redacting the original message in effect removes the message, including all -subsequent edits, from the visible timeline. In this situation, homeservers -will return an empty `content` for the original event as with any other -redacted event. It must be noted that, although they are not immediately -visible in Element, subsequent edits remain unredacted and can be seen via API -calls. See [Future considerations](#future-considerations). +An exception applies to [`GET +/_matrix/client/v3/rooms/{roomId}/event/{eventId}`](https://spec.matrix.org/v1.2/client-server-api/#get_matrixclientv3roomsroomideventeventid), +which should return the *unmodified* event (though the relationship should +still be "bundled", as described +below. -### Server-side aggregation of `m.replace` relationships +#### Server-side aggregation of `m.replace` relationships Note that there can be multiple events with an `m.replace` relationship to a given event (for example, if an event is edited multiple times). Homeservers @@ -284,6 +256,48 @@ be bundled with it (whether or not any subsequent edits are themselves redacted). Note that this behaviour is specific to the `m.replace` relationship. +### Client behaviour + +Clients can often ignore message edit events, since any events the server +returns via the C-S API will be updated by the server to account for subsequent +edits. + +However, clients should apply the replacement themselves (or, alternatively, +request an updated copy of the original via [`GET ++/_matrix/client/v3/rooms/{roomId}/context/{eventId}`](https://spec.matrix.org/v1.2/client-server-api/#get_matrixclientv3roomsroomidcontexteventid) +or similar) when the server is unable to do so. This happens in the following +situations: + +1. The client has already received the original event before the message + edit event arrives. + +2. The original event (and hence its replacement) are encrypted. + +Client authors are reminded to take note of the requirements for [Validity of +message edit events](#validity-of-message-edit-events), and to ignore any +invalid edit events that may be received. + +### Permalinks + +Permalinks to edited events should capture the event ID that the sender is +viewing at that point (which might be an edit ID). The client viewing the +permalink should resolve this ID to the source event ID, and then display the +most recent version of that event. + +### Redactions + +When a message using a `rel_type` of `m.replace` is redacted, it removes that +edit revision. This has little effect if there were subsequent edits, however +if it was the most recent edit, the event is in effect reverted to its content +before the redacted edit. + +Redacting the original message in effect removes the message, including all +subsequent edits, from the visible timeline. In this situation, homeservers +will return an empty `content` for the original event as with any other +redacted event. It must be noted that, although they are not immediately +visible in Element, subsequent edits remain unredacted and can be seen via API +calls. See [Future considerations](#future-considerations). + ## Edge Cases From eba4753627b8fc2fa65f6fa5417ecc267a3da50f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 17 May 2022 18:03:27 +0100 Subject: [PATCH 17/28] Clarify permalinks section --- proposals/2676-message-editing.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index ca5ae46c59..cec4c8f7b4 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -279,10 +279,11 @@ invalid edit events that may be received. ### Permalinks -Permalinks to edited events should capture the event ID that the sender is -viewing at that point (which might be an edit ID). The client viewing the -permalink should resolve this ID to the source event ID, and then display the -most recent version of that event. +Permalinks to edited events should capture the event ID that the creator of the +permalink is viewing at that point (which might be a message edit event). + +The client viewing the permalink should resolve this ID to the original event +ID, and then display the most recent version of that event. ### Redactions From 78550a2c997c7dc7d9a67fedb4efa3270985b248 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 17 May 2022 18:54:16 +0100 Subject: [PATCH 18/28] Notes on edits of replies --- proposals/2676-message-editing.md | 49 +++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index cec4c8f7b4..e983c979be 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -299,23 +299,46 @@ redacted event. It must be noted that, although they are not immediately visible in Element, subsequent edits remain unredacted and can be seen via API calls. See [Future considerations](#future-considerations). +### Edits of replies -## Edge Cases +An event which replaces a [reply](https://spec.matrix.org/v1.2/client-server-api/#rich-replies) might look like this: -How do diffs work on edits if you are missing intermediary edits? - * We just have to ensure that the UI for visualising diffs makes it clear - that diffs could span multiple edits rather than strictly be per-edit-event. +```json +{ + "type": "m.room.message", + "content": { + "body": "> <@richvdh:sw1v.org> ab\n\n * ef", + "msgtype": "m.text", + "format": "org.matrix.custom.html", + "formatted_body": "
In reply to @richvdh:sw1v.org
ab
 * ef", + "m.new_content": { + "body": "ef", + "msgtype": "m.text", + "format": "org.matrix.custom.html", + "formatted_body": "ef" + }, + "m.relates_to": { + "rel_type": "m.replace", + "event_id": "$original_reply_event" + } + } +} +``` + +Note in particular: -What happens when we edit a reply? - * We just send an m.replace which refers to the m.reference target; nothing - special is needed. i.e. you cannot change who the event is replying to. - * The edited reply should ditch the fallback representation of the reply itself - however from `m.new_content` (specifically the `` tag in the - HTML, and the chevron prefixed text in the plaintext which we don't know - whether to parse as we don't know whether this is a reply or not), as we - can assume that any client which can handle edits can also display replies - natively. + * There is no `m.in_reply_to` property in the the `m.relates_to` + object, since it would be redundant (see [Applying + `m.new_content`](#applying-mnew_content) above, which notes that the original + event's `m.relates_to` is preserved), as well as being contrary to the + spirit of + [MSC2674](https://github.com/matrix-org/matrix-spec-proposals/pull/2674) + which expects only one relationship per event. + * `m.new_content` does not contain any ["reply + fallback"](https://spec.matrix.org/v1.2/client-server-api/#fallbacks-for-rich-replies), + since it is assumed that any client which can handle edits can also + display replies natively. ## Future considerations From e58715cfa7e940e58dcd4ecde08283db02f9ee85 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 19 May 2022 09:07:01 +0100 Subject: [PATCH 19/28] Clarify that `m.relates_to` within `m.new_content` is ignored --- proposals/2676-message-editing.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index e983c979be..7ec5cde62e 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -134,7 +134,10 @@ example, an encrypted replacement event might look like this: When applying a replacement, the `content` property of the original event is replaced entirely by the `m.new_content`, with the exception of `m.relates_to`, -which is left *unchanged*. For example, given a pair of events: +which is left *unchanged*. Any `m.relates_to` property within `m.new_content` +is ignored. + +For example, given a pair of events: ```json { From 4c77c01876039076fa4d83c37d1e7a8a07be1203 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Jun 2022 18:30:49 +0100 Subject: [PATCH 20/28] Clarifications from review --- proposals/2676-message-editing.md | 64 +++++++++++++++++-------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index 7ec5cde62e..eccae45dde 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -88,8 +88,12 @@ an event's `content` property, such as `formatted_body`. #### Encrypted events If the original event was encrypted, the replacement should be too. In that -case, `m.new_content` is placed in the `content` of the encrypted payload. For -example, an encrypted replacement event might look like this: +case, `m.new_content` is placed in the `content` of the encrypted payload. The +`m.relates_to` property remains unencrypted, as required by the +[relationships](https://spec.matrix.org/v1.3/client-server-api/#forming-relationships-between-events) +section of the Client-Server API specification. + +For example, an encrypted replacement event might look like this: ```json { @@ -121,15 +125,15 @@ example, an encrypted replacement event might look like this: "m.new_content": { "body": "Hello! My name is bar", "msgtype": "m.text" - }, - "m.relates_to": { - "rel_type": "m.replace", - "event_id": "$some_event_id" } } } ``` +Note that there is no `m.relates_to` property in the encrypted payload. (Any such +property would be ignored.) Likewise, there is no `m.new_content` in the +plaintext body. (Again, any such property would be ignored.) + #### Applying `m.new_content` When applying a replacement, the `content` property of the original event is @@ -202,11 +206,15 @@ of the following criteria must be satisfied: * Neither the replacement nor original events can be state events (ie, neither may have a `state_key`). * The original event must not, itself, have a `rel_type` of `m.replace`. - * The original event and replacement event must have the same `room_id`. * The original event and replacement event must have the same `sender`. * The replacement event (once decrypted, if appropriate) must have an `m.new_content` property. +The original event and replacement event must also have the same `room_id`, as +required by the +[relationships](https://spec.matrix.org/v1.3/client-server-api/#forming-relationships-between-events) +section of the Client-Server API specification. + If any of these criteria are not satisfied, implementations should ignore the replacement event (the content of the original should not be replaced, and the edit should not be included in the server-side aggregation). @@ -265,13 +273,10 @@ Clients can often ignore message edit events, since any events the server returns via the C-S API will be updated by the server to account for subsequent edits. -However, clients should apply the replacement themselves (or, alternatively, -request an updated copy of the original via [`GET -+/_matrix/client/v3/rooms/{roomId}/context/{eventId}`](https://spec.matrix.org/v1.2/client-server-api/#get_matrixclientv3roomsroomidcontexteventid) -or similar) when the server is unable to do so. This happens in the following -situations: +However, clients should apply the replacement themselves when the server is +unable to do so. This happens in the following situations: -1. The client has already received the original event before the message +1. The client has already received and stored the original event before the message edit event arrives. 2. The original event (and hence its replacement) are encrypted. @@ -304,7 +309,24 @@ calls. See [Future considerations](#future-considerations). ### Edits of replies -An event which replaces a [reply](https://spec.matrix.org/v1.2/client-server-api/#rich-replies) might look like this: +Some particular constraints apply to events which replace a +[reply](https://spec.matrix.org/v1.3/client-server-api/#rich-replies). In +particular: + + * There should be no `m.in_reply_to` property in the the `m.relates_to` + object, since it would be redundant (see [Applying + `m.new_content`](#applying-mnew_content) above, which notes that the original + event's `m.relates_to` is preserved), as well as being contrary to the + spirit of + [MSC2674](https://github.com/matrix-org/matrix-spec-proposals/pull/2674) + which expects only one relationship per event. + + * `m.new_content` should **not** contain any ["reply + fallback"](https://spec.matrix.org/v1.3/client-server-api/#fallbacks-for-rich-replies), + since it is assumed that any client which can handle edits can also + display replies natively. + +An example of an edit to a reply is as follows: ```json { @@ -328,20 +350,6 @@ An event which replaces a [reply](https://spec.matrix.org/v1.2/client-server-api } ``` -Note in particular: - - * There is no `m.in_reply_to` property in the the `m.relates_to` - object, since it would be redundant (see [Applying - `m.new_content`](#applying-mnew_content) above, which notes that the original - event's `m.relates_to` is preserved), as well as being contrary to the - spirit of - [MSC2674](https://github.com/matrix-org/matrix-spec-proposals/pull/2674) - which expects only one relationship per event. - - * `m.new_content` does not contain any ["reply - fallback"](https://spec.matrix.org/v1.2/client-server-api/#fallbacks-for-rich-replies), - since it is assumed that any client which can handle edits can also - display replies natively. ## Future considerations From 1850fb5906a031996b404e2dc1aa3b22b43b770b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Jun 2022 18:34:15 +0100 Subject: [PATCH 21/28] event ids are sorted lexicographically --- proposals/2676-message-editing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index eccae45dde..73462cd492 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -225,7 +225,7 @@ Whenever a homeserver would return an event via the Client-Server API, it should check for any valid, applicable `m.replace` event, and if one is found, it should first modify the `content` of the original event according to the `m.new_content` of the most recent edit (as determined by -`origin_server_ts`, falling back to `event_id`). +`origin_server_ts`, falling back to a lexicographic ordering of `event_id`). An exception applies to [`GET /_matrix/client/v3/rooms/{roomId}/event/{eventId}`](https://spec.matrix.org/v1.2/client-server-api/#get_matrixclientv3roomsroomideventeventid), From c9e6652721c0ab6987e87cd6165d5671060202a9 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Jun 2022 18:47:27 +0100 Subject: [PATCH 22/28] Clarify aggregation section --- proposals/2676-message-editing.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index 73462cd492..d5c21eddcb 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -236,11 +236,13 @@ below. #### Server-side aggregation of `m.replace` relationships Note that there can be multiple events with an `m.replace` relationship to a -given event (for example, if an event is edited multiple times). Homeservers -should aggregate `m.replace` relationships as in -[MSC2675](https://github.com/matrix-org/matrix-doc/pull/2675). The aggregation +given event (for example, if an event is edited multiple times). These should +be [aggregated](https://spec.matrix.org/v1.3/client-server-api/#aggregations) +by the homeserver. + +The format of the aggregation for `m.replace` simply gives gives the `event_id`, `origin_server_ts`, and `sender` of the most recent -replacement event (as determined by `origin_server_ts`). +replacement event (determined as above). This aggregation is bundled into the `unsigned/m.relations` property of any event that is the target of an `m.replace` relationship. For example: From 1e63094d3294eddf8ee45c7d38fa1c71f6a7294b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 12 Jul 2022 14:49:46 +0100 Subject: [PATCH 23/28] minor clarifications --- proposals/2676-message-editing.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index d5c21eddcb..dd449ab371 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -134,6 +134,9 @@ Note that there is no `m.relates_to` property in the encrypted payload. (Any suc property would be ignored.) Likewise, there is no `m.new_content` in the plaintext body. (Again, any such property would be ignored.) +For clarity: the payload must be encrypted as normal, ratcheting the Megolm session +as normal. The original Megolm ratchet entry should **not** be re-used. + #### Applying `m.new_content` When applying a replacement, the `content` property of the original event is @@ -257,7 +260,7 @@ event that is the target of an `m.replace` relationship. For example: "m.replace": { "event_id": "$latest_edit_event_id", "origin_server_ts": 1649772304313, - "sender": "@editing_user:localhost + "sender": "@editing_user:localhost" } } } From 23738736655b8d78bba2e7e61d8b93d6b18cf535 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 12 Jul 2022 22:21:31 +0100 Subject: [PATCH 24/28] Clarify which endpoints support edits --- proposals/2676-message-editing.md | 36 ++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index dd449ab371..f65f8c28ee 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -224,18 +224,6 @@ edit should not be included in the server-side aggregation). ### Server behaviour -Whenever a homeserver would return an event via the Client-Server API, it -should check for any valid, applicable `m.replace` event, and if one is found, it -should first modify the `content` of the original event according to the -`m.new_content` of the most recent edit (as determined by -`origin_server_ts`, falling back to a lexicographic ordering of `event_id`). - -An exception applies to [`GET -/_matrix/client/v3/rooms/{roomId}/event/{eventId}`](https://spec.matrix.org/v1.2/client-server-api/#get_matrixclientv3roomsroomideventeventid), -which should return the *unmodified* event (though the relationship should -still be "bundled", as described -below. - #### Server-side aggregation of `m.replace` relationships Note that there can be multiple events with an `m.replace` relationship to a @@ -272,6 +260,30 @@ be bundled with it (whether or not any subsequent edits are themselves redacted). Note that this behaviour is specific to the `m.replace` relationship. +#### Server-side replacement of content + +Whenever an `m.replace` is to be bundled with an event as above, the server should +also modify the `content` of the original event according +to the `m.new_content` of the most recent edit (as determined by +`origin_server_ts`, falling back to a lexicographic ordering of `event_id`). + +An exception applies to [`GET +/_matrix/client/v3/rooms/{roomId}/event/{eventId}`](https://spec.matrix.org/v1.2/client-server-api/#get_matrixclientv3roomsroomideventeventid), +which should return the *unmodified* event (though the relationship should +still be bundled, as described above. + +The endpoints where this behaviour takes place is the same as those where +aggregations are bundled, with the exception of +`/room/{roomId}/event/{eventId}`. This includes: + + * `GET /rooms/{roomId}/messages` + * `GET /rooms/{roomId}/context/{eventId}` + * `GET /rooms/{roomId}/relations/{eventId}` + * `GET /rooms/{roomId}/relations/{eventId}/{relType}` + * `GET /rooms/{roomId}/relations/{eventId}/{relType}/{eventType}` + * `GET /sync` when the relevant section has a `limited` value of `true` + * `POST /search` for any matching events under `room_events`. + ### Client behaviour Clients can often ignore message edit events, since any events the server From 8ec4cf3a14f3ba290c56cee94917ad521645d53e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 12 Jul 2022 22:24:32 +0100 Subject: [PATCH 25/28] move definition of latest edit --- proposals/2676-message-editing.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index f65f8c28ee..7a1b916d49 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -231,9 +231,10 @@ given event (for example, if an event is edited multiple times). These should be [aggregated](https://spec.matrix.org/v1.3/client-server-api/#aggregations) by the homeserver. -The format of the aggregation for `m.replace` simply gives -gives the `event_id`, `origin_server_ts`, and `sender` of the most recent -replacement event (determined as above). +The format of the aggregation for `m.replace` simply gives gives the +`event_id`, `origin_server_ts`, and `sender` of the most recent replacement +event (as determined by `origin_server_ts`, falling back to a lexicographic +ordering of `event_id`). This aggregation is bundled into the `unsigned/m.relations` property of any event that is the target of an `m.replace` relationship. For example: @@ -264,8 +265,7 @@ relationship. Whenever an `m.replace` is to be bundled with an event as above, the server should also modify the `content` of the original event according -to the `m.new_content` of the most recent edit (as determined by -`origin_server_ts`, falling back to a lexicographic ordering of `event_id`). +to the `m.new_content` of the most recent edit (determined as above). An exception applies to [`GET /_matrix/client/v3/rooms/{roomId}/event/{eventId}`](https://spec.matrix.org/v1.2/client-server-api/#get_matrixclientv3roomsroomideventeventid), From 08489e828090dbc3adb4419dd8b2cc5e14f4f682 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 13 Jul 2022 07:35:03 +0100 Subject: [PATCH 26/28] Apply suggestions from code review Co-authored-by: Hubert Chathi --- proposals/2676-message-editing.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index 7a1b916d49..1c23710dab 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -132,7 +132,7 @@ For example, an encrypted replacement event might look like this: Note that there is no `m.relates_to` property in the encrypted payload. (Any such property would be ignored.) Likewise, there is no `m.new_content` in the -plaintext body. (Again, any such property would be ignored.) +cleartext body. (Again, any such property would be ignored.) For clarity: the payload must be encrypted as normal, ratcheting the Megolm session as normal. The original Megolm ratchet entry should **not** be re-used. @@ -270,7 +270,7 @@ to the `m.new_content` of the most recent edit (determined as above). An exception applies to [`GET /_matrix/client/v3/rooms/{roomId}/event/{eventId}`](https://spec.matrix.org/v1.2/client-server-api/#get_matrixclientv3roomsroomideventeventid), which should return the *unmodified* event (though the relationship should -still be bundled, as described above. +still be bundled, as described above). The endpoints where this behaviour takes place is the same as those where aggregations are bundled, with the exception of From f3d328d2aa9f01b9dc81685284c0736390e3be3b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 13 Jul 2022 10:56:09 +0100 Subject: [PATCH 27/28] fix typo --- proposals/2676-message-editing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index 1c23710dab..83da0bebe7 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -38,7 +38,7 @@ now other than formalise the status quo. This MSC, along with the others mentioned above, therefore seeks primarily to do that. Although there is plenty of scope for improvement, we consider that -better done in *future* MSCs, based on a shared understaning of the *current* +better done in *future* MSCs, based on a shared understanding of the *current* implementation. In short, this MSC prefers fidelity to the current implementations over From b2457619ab3ac6199598d05a5e1b33dc51ab3ee1 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 13 Jul 2022 10:59:01 +0100 Subject: [PATCH 28/28] Attempt to clarify encrypted events --- proposals/2676-message-editing.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/proposals/2676-message-editing.md b/proposals/2676-message-editing.md index 83da0bebe7..680563b501 100644 --- a/proposals/2676-message-editing.md +++ b/proposals/2676-message-editing.md @@ -130,9 +130,11 @@ For example, an encrypted replacement event might look like this: } ``` -Note that there is no `m.relates_to` property in the encrypted payload. (Any such -property would be ignored.) Likewise, there is no `m.new_content` in the -cleartext body. (Again, any such property would be ignored.) +Note that: + * There is no `m.relates_to` property in the encrypted payload. (Any such + property would be ignored.) + * There is no `m.new_content` property in the cleartext `content` of the + `m.room.encrypted` event. (Again, any such property would be ignored.) For clarity: the payload must be encrypted as normal, ratcheting the Megolm session as normal. The original Megolm ratchet entry should **not** be re-used.