-
Notifications
You must be signed in to change notification settings - Fork 380
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
MSC2589: Improve replies #2589
MSC2589: Improve replies #2589
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
# MSC2589: Improving replies | ||
|
||
Currently replies in the spec require clients to parse HTML and/or plain text in order to accurately | ||
represent the event. This fallback also introduces a number of limitations preventing clients from | ||
typically being able to reply to non-text events, such as state events or images. | ||
|
||
This proposal alters the requirements for replies such that they can be expanded to cover any event | ||
type, and be easier for clients to render. | ||
|
||
For context, the current spec is extensively documented here: https://matrix.org/docs/spec/client_server/r0.6.1#rich-replies | ||
|
||
This fixes https://github.com/matrix-org/matrix-doc/issues/1541 and would technically address | ||
https://github.com/matrix-org/matrix-doc/issues/1661 as well due to not defining a strict format. | ||
|
||
## Proposal | ||
|
||
Firstly, building off the concepts of [MSC1849](https://github.com/matrix-org/matrix-doc/pull/1849) | ||
the relationship between events in the context of replies changes to be: | ||
|
||
```json | ||
{ | ||
"m.relationship": { | ||
"rel_type": "m.reference", | ||
"event_id": "$example" | ||
} | ||
} | ||
``` | ||
|
||
The fallback for replies is simply replaced by a best effort clause: clients SHOULD do their | ||
best to provide context of what happened in the `body` and `formatted_body` for clients which | ||
do not support replies. This could take the form of a simple blockquote, similar to quoting a | ||
message. | ||
|
||
Instead of having to parse the fallback to get the reply, clients MUST send a `reply_body` | ||
(and optionally a `reply_formatted_body`) in the event content to represent what the user is | ||
replying with. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would this relate to edits, which also send a new_content, effectively duplicating the {
"content": {
"body": "...",
"formatted_body": "...",
"reply_body": "...",
"reply_formatted_body": "..."
},
"new_content": {
"body": "...",
"formatted_body": "...",
"reply_body": "...",
"reply_formatted_body": "..."
}
} That's a bit ridiculous, I think. You may be able to drop the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Edits are a problem for 1849 entirely. Whichever MSC lands first will force the other to figure it out :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While that is true, I think it is a valid concern, that we do have a somewhat exponential trend of message contents here :D There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For simplicity I'd be inclined to say that edits, under 1849, would include the fallback and other properties so clients can simply map it over top and not worry about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
|
||
In an example, if someone were to send a message like "Version 1.1.0 is out!" and another were | ||
to reply to it, the reply's content would look like this: | ||
|
||
```json | ||
{ | ||
"body": "> Version 1.1.0 is out!\n\nAwesome, thanks for the hard work!", | ||
"format": "org.matrix.custom.html", | ||
"formatted_body":"<blockquote>Version 1.1.0 is out!</blockquote><p>Awesome, thanks for the hard work!</p>", | ||
"reply_body": "Awesome, thanks for the hard work!", | ||
"reply_fomatted_body": "<p>Awesome, thanks for the hard work!</p>", | ||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"m.relationship": { | ||
"rel_type": "m.reference", | ||
"event_id": "$example" | ||
} | ||
} | ||
``` | ||
|
||
The `format` field describes the `reply_formatted_body` format. The requirement for replies to | ||
be HTML-formatted is also dropped: a plain text reply (no `format`) is completely acceptable. | ||
|
||
## Potential issues | ||
|
||
Events will be larger as a result, though typical messaging scenarios hardly get anywhere near the | ||
event size limit. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I don't think the event size limit is an issue, replies currently already have quite a bit of overhead already and this would do not that much to decrease it. If someone sends the draft for an MSC for example, replying to it basically sends the same message again in size. Since Riot is already a considerable factor of my mobile data usage, I think the alternative you propose is a better solution for at least the bloat issue. |
||
|
||
There is also an opportunity for the fallback and the `reply_*` fields to desync, though this is | ||
largely believed to be a client bug rather than a fault of the spec. | ||
|
||
Some events, like images, cannot be represented easily as text. Clients could show something like | ||
"> regarding Bob's image: release-timeline.png" instead. | ||
|
||
## Alternatives | ||
|
||
We could ditch the idea of a fallback entirely, leaving clients which don't support the feature | ||
in the dust. This is a viable option for a number of use cases (as the reply and the event are | ||
typically within moments of each other), however many replies that existing users make are done | ||
so with no referencing context. For example, "does anyone know how to fix this?" means nothing if | ||
the original event's context isn't presented. This is largely an issue for bridges and other simple | ||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||
clients, though many projects which realize the reply fallback is not perfect tend to take action | ||
to support replies properly. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I would prefer this. You don't lose that much context, if you are replying without a large gap to a message and if you do, you may want to use a client, that supports replies. Many bridges already need to deal with replies too and for at least a few of them, the fallback is actually a problem and most of them strip the fallback to replace it with their own. The IRC bridge for example would do much better to send their own fallback, so that messages don't become too long, which many IRC users dislike. Other platforms like WhatsApp for example do replies differently, so the fallback provides no value. It is far easier to add a fallback in my experience, than to strip it. I'd also argue, that Riot/Android or RiotX would have implemented proper replies by now, if there was no reliable fallback. Usually the experience of proper replies is far better than relying on the fallback and I think client devs should be nudged to provide the better experience, when possible. Stripping the fallback also takes quite a bit more effort than adding it, if you don't want to implement replies. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't have to strip the fallback with this proposal. The IRC bridge also already adds a fallback itself, in the form of supporting replies properly by requesting the event it references. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After re-reading relevant parts of #1849 and the discussion around them I tend to agree that we have to ditch the reply fallback in the form it exists now. Expecting too much from clients, the fallback text construction rules ended up being the second-class citizen and slip in all kinds of holes with respect to edits, redactions, E2EE; and worst of all, the list of holes is open to future additions. From the client perspective, once we moved to all these additional things that have to interact with replies, I see no way to provide consistent fallback text experience to clients that don't know about replies (meaning, specifically: neither parse the reply text, nor support replies through requesting the event replied to). Clients that do know about replies, do either of these two things anyway. In order to give at least some help on "what was your comment about" thing, we could probably leave "In reply to |
||
|
||
## Security considerations | ||
|
||
https://github.com/matrix-org/matrix-doc/issues/1654 is still not fixed as part of this. | ||
|
||
## Unstable prefix | ||
|
||
Clients should use `org.matrix.msc2589` in place of `m.*` within this MSC. For example, | ||
`m.relationship` becomes `org.matrix.msc2589.relationship` while this MSC has not been included in | ||
a spec release. | ||
KitsuneRal marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why
m.reference
and not something likem.reply
? Reference seems kinda too generalized, an edit is a reference, a reaction is a reference, you might be referencing multiple things in your content but only replying to one, etc.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stolen as-defined from 1849. That's about the end of my justification 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to trigger bikeshedding but I'm also not entirely convinced by the relationship name and I'd prefer to have
m.reply
even if it happens to be unnecessarily specific later on (which I believe is unlikely; see also what happened toIn-Reply-To
andReferences
in e-mail).MSC1849 mentions threading as a possible reason for the name; but label-based threading proposed by #2326 doesn't use
m.reference
(and, arguably,m.reference
would anyway look alien there as well).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move the bikeshed to 1849, which this MSC can follow along with. If this MSC somehow gets closer to FCP than 1849, we can do some course correction.