Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

MSC3925: m.replace aggregation with full event #3925

Merged
merged 34 commits into from
Feb 21, 2023
Merged
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
116eb4e
init m.replace aggregation with full event
benkuly Nov 3, 2022
88c434d
Rename tmp.md to 3925-replace-aggregation-with-full-event.md
benkuly Nov 3, 2022
5a54a03
Update 3925-replace-aggregation-with-full-event.md
benkuly Nov 3, 2022
a065b00
Update 3925-replace-aggregation-with-full-event.md
benkuly Nov 3, 2022
4f5ecde
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 4, 2022
6ea5486
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 4, 2022
09602eb
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 8, 2022
3876a46
redact instead of delete
benkuly Nov 8, 2022
97dfdce
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 8, 2022
c0ec33d
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 8, 2022
f7fa717
Merge branch 'main' of github.com:benkuly/matrix-spec-proposals
benkuly Nov 8, 2022
0bba4e4
word wrap 80
benkuly Nov 8, 2022
db0ca3e
remove immutable argument
benkuly Nov 8, 2022
ff6fc37
add alternative from https://github.com/matrix-org/matrix-spec/issues…
benkuly Nov 8, 2022
291b921
add json example
benkuly Nov 8, 2022
8a4f38f
shorter version of the actual proposal
benkuly Nov 8, 2022
e365e01
describe the actual reason, why encrypted events cannot be replaced
benkuly Nov 8, 2022
05ecbe5
mention discussion
benkuly Nov 8, 2022
919edb5
typo
benkuly Nov 8, 2022
8fb8814
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
eac5551
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
5aced11
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
fe75f9c
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
767acad
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
e3a461e
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
b84a03a
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
5bfffb4
clarify inconsistent behavior with replaced contents
benkuly Nov 11, 2022
1ea94a2
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
fae2c26
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 15, 2022
ee0dffa
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 15, 2022
fd37307
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 15, 2022
efc6060
clarify inconsistent behavior and put it into introduction
benkuly Nov 15, 2022
64da169
remove superfluous event content field
benkuly Feb 8, 2023
3d9e2ab
add some details to potential issues
benkuly Feb 8, 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
140 changes: 140 additions & 0 deletions proposals/3925-replace-aggregation-with-full-event.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
# MSC3925: m.replace aggregation with full event

As [currently specified](https://spec.matrix.org/v1.4/client-server-api/#server-side-replacement-of-content),
servers should replace the content of events that have been replaced via an `m.replace`
relation.

There are some issues with this requirement:

* Changing the fundamental concept of mostly immutable events is confusing. The server
can respond with different event contents for the same `event_id`.
* If an event with `m.replace` relation is redacted, clients need to
detect if the original content was replaced, and possibly need to fetch the
original content.
* Servers cannot replace the original event content for encrypted events (because the
replacement content is inside the encrypted body).
See [matrix-spec#1299](https://github.com/matrix-org/matrix-spec/issues/1299).
* Replacing the content can lead to inconsistent behavior on clients which don't
support replacing events.
Assume that we have this timeline on the server:
- `{"event_id": "E1", "content": {"body": "1"}}`
- `{"event_id": "E2", "content": {"body": "* 2", "m.new_content": {"body": "2"}}` // replaces `E1`

A `/sync` is done after `E2`. We will have this timeline on a client:
- `{"event_id": "E1", "content": {"body": "2"}}`
- `{"event_id": "E2", "content": {"body": "* 2"}`

Although `2` is a replaced body it does not have a `*`. This looks to the
user as if it is the original content of the event.


## Proposal

The following two changes are proposed:
1. The [server-side aggregation of `m.replace` relationships](https://spec.matrix.org/v1.4/client-server-api/#server-side-aggregation-of-mreplace-relationships)
is extended to be the entire content of the most recent replacement event, formatted
as described in [Room Event Format](https://spec.matrix.org/v1.4/client-server-api/#room-event-format).
This ensures that the client will always have the most recent edit without having to
fetch it from the server.
turt2live marked this conversation as resolved.
Show resolved Hide resolved
2. Servers should no longer replace the original content of an event as described
at https://spec.matrix.org/v1.4/client-server-api/#server-side-replacement-of-content.

For an original event:

```json5
{
"event_id": "$original_event",
"type": "m.room.message",
"content": {
"body": "I really like cake",
"msgtype": "m.text",
"formatted_body": "I really like cake"
}
// irrelevant fields not shown
}

```

With a replacing event:

```json5
{
"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"
},
"m.relates_to": {
"rel_type": "m.replace",
"event_id": "$original_event_id"
}
}
// irrelevant fields not shown
}

```

This is how the original event would look like after the replacement:

```json5
{
"event_id": "$original_event",
"type": "m.room.message",
"content": {
"body": "I really like cake",
"msgtype": "m.text",
"formatted_body": "I really like cake"
},
"unsigned": {
"m.relations": {
"m.replace": {
"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"
},
"m.relates_to": {
"rel_type": "m.replace",
"event_id": "$original_event_id"
}
}
// irrelevant fields not shown
}
}
}
// irrelevant fields not shown
}

```

## Potential issues

* There could be clients which rely on the current behavior:
* element-web relied on it until [matrix-org/matrix-js-sdk#3045](https://github.com/matrix-org/matrix-js-sdk/pull/3045)
* The failure mode is: suppose we have an event E, which was subsequently replaced by an event E'. Now, if jumped back to a bit of timeline that contains E but not E', then clients unaware of this change will show the original event E rather than the edited content E'. But: That is an edge-case and it already happens for events other than `m.room.message` (including encrypted events) due to [matrix-org/synapse#12503](https://github.com/matrix-org/synapse/issues/12503).
* It will be harder for clients which do not support replacing events to get
the current content of an event: currently they can just look at `content.body`.
However, there is an inconsistent behavior for clients that do not support
replacing events (see above).

## Alternatives

One [suggestion](https://github.com/matrix-org/matrix-spec/issues/1299#issuecomment-1290332433) is
to have the server return the complete replacement event instead of the original event. However, that would
break compatibility with existing clients and is a more invasive change.

## Security considerations

benkuly marked this conversation as resolved.
Show resolved Hide resolved
None currently foreseen.

## Unstable prefix

No unstable prefix is currently proposed.