Add rich replies #1617

turt2live · 2018-08-30T04:58:24Z

Rendered: see 'docs' status check

This is the spec PR for MSC1234.

The notable parts of this are:

The titles go to insane levels. Rich replies are fairly complex and need some splitting apart to be understandable.
The allowed HTML tags now have an exception for <mx-reply>

Please note that the event example is intended to be fixed by a PR that fixes all event examples.

Fixes matrix-org#1234 The notable parts of this are: * The titles go to insane levels. Rich replies are fairly complex and need some splitting apart to be understandable. * The allowed HTML tags now have an exception for `<mx-reply>` Please note that the event example is intended to be fixed by a PR that fixes all event examples.

richvdh · 2018-08-31T12:35:22Z

specification/modules/instant_messaging.rst

+better context for the conversation being had. This sort of embedding another
+message in a message is known as a "rich reply", or occasionally just a "reply".
+
+Rich replies may reference another event which also has a rich reply, infinitely.


can you put this at the end of the paragraph rather than the start? it makes it sound like the rest of the paragraph is going to be about replies which reference replies.

richvdh · 2018-08-31T12:36:07Z

specification/modules/instant_messaging.rst

+defined is a "rich reply" where a user may reference another message to create a
+thread-like conversation.
+
+Relationships are defined under an ``m.relates_to`` key in the event's ``content``.


is the structure for this key spelt out anywhere? I'm initially assuming it's an event id but later discovering that it's probably more complicated than that.

it ends up looking like this:

"m.relates_to": { "m.in_reply_to": { "event_id": "$thing" } }

... will add that in somewhere.

richvdh · 2018-08-31T12:39:27Z

specification/modules/instant_messaging.rst

+Rich replies may reference another event which also has a rich reply, infinitely.
+A rich reply is formed through use of an ``m.relates_to`` relation for ``m.in_reply_to``
+where a single key, ``event_id``, is used to reference the event being replied to.
+The referenced event ID MUST belong to the same room where the reply is being sent.


Sadly it's hard to enforce this sort of thing for events received over federation, so I'd actually soften this to a SHOULD and say that clients should be prepared for it to be otherwise. (likewise the event id being a bad format or indeed any of m.relates_to having a funny structure).

richvdh · 2018-08-31T12:41:15Z

specification/modules/instant_messaging.rst

+
+Clients that do support rich replies MUST provide the fallback format on replies,
+and MUST strip the fallback before rendering the reply. Rich replies MUST supply
+a ``format`` of ``org.matrix.custom.html`` and therefore a ``formatted_body``


oh now we've baked org.matrix.custom.html into the spec : (.

(Really we didn't need to, replies easily could have been constructed out of some keys in content instead and worked as plaintext)

@Half-Shot its only required because of the requirement for a fallback, moving stuff into separate keys won't make old clients magically understand it.

In fairness, some clients reject the HTML because of the non-standard weird tag. Also, it's a pain in the arse to parse out the reply text from the bodies so the keys would still have been appreciated.

ftr the baking in happened a long time ago relative to this PR.

And I hate the fact that we are speccing in something that is nasty rather than adding a key and asking client's to adopt it.

I'm all for dropping the fallback on the floor ENTIRELY
the decision was above me

That sounds like a breach of the HTML spec which says you should just ignore invalid tags but not their children?

The IRC bridge (wrongly, imo) assumes that unknown tags mean the rendering will end up looking wierd and falls back to plain.

While I appreciate that the fallback isn't the best case scenario, it's what we have and what we're stuck with for the time being. Clients encounter this format on a regular basis now, and it'll require a bit more effort to fix than we have bandwidth for on this PR. I'd highly recommend an MSC proposing a change.

Fair, not blaming anyone. I just wish we got it right this time around rather than being stuck with it until r0.5

richvdh · 2018-08-31T12:41:34Z

specification/modules/instant_messaging.rst

+Clients that do support rich replies MUST provide the fallback format on replies,
+and MUST strip the fallback before rendering the reply. Rich replies MUST supply
+a ``format`` of ``org.matrix.custom.html`` and therefore a ``formatted_body``
+alongside the ``body`` and appropriate ``msgtype``. The specific fall back text


fallback (no space)

richvdh · 2018-08-31T12:42:11Z

specification/modules/instant_messaging.rst

+as if rich replies were not special.
+
+Clients that do support rich replies MUST provide the fallback format on replies,
+and MUST strip the fallback before rendering the reply. Rich replies MUST supply


s/supply/have/

richvdh · 2018-08-31T12:49:35Z

specification/modules/instant_messaging.rst

+  This is where the reply goes
+
+
+The ``formatted_body`` ends up using the following template:


s/ends up using/should use/ ?

richvdh · 2018-08-31T12:55:00Z

specification/modules/instant_messaging.rst

+
+The related event's ``body`` would be a file name, which may not be very descriptive.
+The related event should additionally not have a ``format`` or ``formatted_body``
+in the ``content`` - if the event does, it should be ignored. Because the filename


"it" == body ? Can you say so?

this is actually describing the format and formatted_body - will clarify

t3chguy · 2018-08-31T14:50:03Z

specification/modules/instant_messaging.rst

+Rich replies can only be constructed in the form of ``m.room.message`` events with
+a ``msgtype`` of ``m.text`` or ``m.notice``. Due to the fallback requirements, rich
+replies cannot be constructed for types of ``m.emote``, ``m.file``, etc. Rich replies
+may reference any other ``m.room.message`` event, however.


~~well I mean they can't, as you can't provide a nice fallback say if you're replying to a video.~~

I forgot this was loosened to just text

this is also talking about referencing, not the actual reply itself. It's entirely possible to hit reply on an image, so long as you send text and not another image.

richvdh

lgtm otherwise

richvdh · 2018-08-31T17:02:21Z

specification/modules/instant_messaging.rst

-held in the ``content``.
+held in the ``content``, like in the following example::
+
+  {


I'm afraid I don't find this example adds any value.

turt2live requested a review from a team August 30, 2018 04:58

Changelog

38bb222

richvdh requested changes Aug 31, 2018

View reviewed changes

t3chguy reviewed Aug 31, 2018

View reviewed changes

turt2live added 2 commits August 31, 2018 10:08

Merge remote matrix-org/master

9004be7

Add additional clarity to how rich replies are structured and used

567843e

turt2live requested a review from richvdh August 31, 2018 16:20

richvdh approved these changes Aug 31, 2018

View reviewed changes

Take out unhelpful example

4b9fb99

turt2live merged commit 25a461b into matrix-org:master Aug 31, 2018

turt2live deleted the travis/c2s/rich-replies branch August 31, 2018 17:21

tysonclugg mentioned this pull request Dec 10, 2018

Threaded Messaging element-hq/element-web#2349

Closed

richvdh mentioned this pull request Dec 27, 2018

Event Quoting (SPEC-432) #693

Closed

MadLittleMods mentioned this pull request Sep 14, 2021

Message editing: attach message as reply to other message via Edit form element-hq/element-meta#1047

Open

DanilaFe mentioned this pull request Jan 23, 2022

Proper room management DanilaFe/matrix-highlight#19

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add rich replies #1617

Add rich replies #1617

turt2live commented Aug 30, 2018

richvdh Aug 31, 2018

richvdh Aug 31, 2018

turt2live Aug 31, 2018

richvdh Aug 31, 2018

richvdh Aug 31, 2018

Half-Shot Aug 31, 2018

t3chguy Aug 31, 2018

Half-Shot Aug 31, 2018

turt2live Aug 31, 2018

t3chguy Aug 31, 2018

Half-Shot Aug 31, 2018

turt2live Aug 31, 2018

Half-Shot Aug 31, 2018

richvdh Aug 31, 2018

richvdh Aug 31, 2018

richvdh Aug 31, 2018

richvdh Aug 31, 2018

richvdh Aug 31, 2018

turt2live Aug 31, 2018

t3chguy Aug 31, 2018 •

edited

Loading

turt2live Aug 31, 2018

richvdh left a comment

richvdh Aug 31, 2018

		This is where the reply goes


		The ``formatted_body`` ends up using the following template:

Add rich replies #1617

Add rich replies #1617

Conversation

turt2live commented Aug 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t3chguy Aug 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t3chguy Aug 31, 2018 •

edited

Loading