Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

MSC2589: Improve replies #2589

Closed
wants to merge 3 commits into from
Closed

MSC2589: Improve replies #2589

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
47ee0a5
Proposal to improve replies
turt2live May 29, 2020
04e2827
Assign MSC number
turt2live May 29, 2020
40a0418
Clarify that HTML is no longer required.
turt2live May 29, 2020
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
87 changes: 87 additions & 0 deletions proposals/2589-reply-improvements.md
View file
Open in desktop
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",
Copy link
Contributor

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 like m.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.

Copy link
Member Author

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 😄

Copy link
Member

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 to In-Reply-To and References 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).

Copy link
Member Author

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.

"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.
Copy link
Contributor

Choose a reason for hiding this comment

The 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 body and formatted_body? If you don't want to go with your proposed alternative, would I need to send something like this?:

{
  "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 reply_* parts in new_content though, but I would still prefer to not duplicate content unnecessarily.

Copy link
Member Author

Choose a reason for hiding this comment

The 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 :)

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

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

The m.new_content in edits currently doesn't include a reply fallback at all ftr


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.
Copy link
Contributor

Choose a reason for hiding this comment

The 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.
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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 <a href="https://matrix.to/#/$eventid">event</a>" as the least troublesome part of the reply, in formatted_body; and maybe Replying to @mxid:example.org:\n prepended to body. No body pieces of the message replied to, and no display names; @mxid:example.org can still be abusive though, so I'm not certain about the plain text fallback.


## 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