MSC3746: Render image data in reactions

[AndrewRyanChama]

Rendered: https://github.com/AndrewRyanChama/matrix-doc/blob/archama/imagereacts/proposals/3746-image-reactions.md
Implementation: matrix-org/matrix-react-sdk#7903
Signed-off-by: Andrew Ryan [email protected]

---

Please take a look and sanity check this approach is feasible. I'm personally not worried about it, but decoupling what is displayed in the reaction with the aggregation key opens the door to potentially annoying situations.

If this isn't feasible, I can put together a proposal that rams everything into the aggregation key, but I don't like that very much either.

[turt2live]

@AndrewRyanChama when you get the chance, please sign off on the changes so we can include them down the line :)

[Sorunome]

Just wanting to point out that the name identifier of emotes, the :emote: thing, could be the same for different emotes. As you put this into the key attribute here, synapse will think that it was the same reaction and "deduplicate" it, even if a different image. More appropriately would be to put the mxc url into the key attribute, as that is guaranteed to be unique.

[sumnerevans]

I agree with this. I think that the fallback should be in a shortcode key in the content:

"content": {
  "m.relates_to": {
    "rel_type": "m.annotation",
    "event_id": "$abcdefg",
    "key": "mxc://matrix.org/VOczFYqjdGaUKNwkKsTjDwUa"
  },
  "shortcode": ":partyparrot:",
  "info": {
    // same
  }
}

For the record, Beeper has been doing this with our Slack and Discord bridges for a while now, and it works well.

The downside is that the fallback is annoying for clients that don't support it, but that's better than having reactions incorrectly grouped.

[ajbura]

We’ve also implemented this approach in Cinny [1].

[HarHarLinks]

As this is related to images, wouldn't it make sense to adhere to using the body field for fallbacks, like images currently use it, instead of introducing a new field shortcode?
https://spec.matrix.org/latest/client-server-api/#mimage

Is it worth stating in spec that the m.relates_to.key is NOT to be used to fetch the image, but the required url field?

[sumnerevans]

I have created #4027 which intends to document what the implementations that already exist do.

[sumnerevans]

I agree with this. I think that the fallback should be in a shortcode key in the content:

"content": {
  "m.relates_to": {
    "rel_type": "m.annotation",
    "event_id": "$abcdefg",
    "key": "mxc://matrix.org/VOczFYqjdGaUKNwkKsTjDwUa"
  },
  "shortcode": ":partyparrot:",
  "info": {
    // same
  }
}

For the record, Beeper has been doing this with our Slack and Discord bridges for a while now, and it works well.

The downside is that the fallback is annoying for clients that don't support it, but that's better than having reactions incorrectly grouped.

[HarHarLinks]

I think this is generally a good idea which with some small tweaks has proven successful in the wild already.

[HarHarLinks]

Suggested change

	In order to describe a reaction with an image, we simply include an mxc url and optionally [ImageInfo](https://github.com/matrix-org/matrix-doc/blob/f8b83b7fb1194ab48ee3461185c4764ebbfecc68/data/event-schemas/schema/core-event-schema/msgtype_infos/image_info.yaml) in the event content
	In order to describe a reaction with an image, we simply include an mxc url and optionally [ImageInfo](https://github.com/matrix-org/matrix-spec/blob/main/data/event-schemas/schema/core-event-schema/msgtype_infos/image_info.yaml) in the event content.

Perhaps this should state that the m.image spec is to be mirrored here. As commented elsewhere, optionally(?) including body as fallback, again the same as m.image, can be a good choice.

In what regard is ImageInfo useful to have here? Does any client implement it at this point? Maybe file type.
I think thumbnail won't be used anyway (unless your client uses it instead to reduce file size downloaded over cell when mobile? not sure that's realistic - rather display fallback shortcode) and dimensions don't matter as it will be scaled to fit into the reaction display area.

[HarHarLinks]

Perhaps the following process should be stated in order to define how to interpret these extended reaction events:

1. check presence of url field. If found we have a custom image reaction.
   1. read image info if present and apply any special logic(?)
2. read fallback from body (or shortcode, see discussion above), which can be used as fallback in case the image does not load/is otherwise invalid
3. use m.relates_to.key as MSC2677 literal reaction as usual, which is also the fallback for clients not implementing MSC3746.

[HarHarLinks]

I'm wondering if any considerations in hindsight to MSC2545: Image Packs make sense, I suppose it could easily get tacked on in hindsight.

E.g. "image_pack_source" could point to the source of the image pack used when the sender chooses to include it (e.g. it is a public image pack room), allowing others to discover image packs and start using them.

[uhoreg]

Please wrap your lines to ~80 to ~120 characters.

[uhoreg]

I think it would be difficult for clients to ensure that they use a key that does not collide with other reactions.

Would it be sufficient to tell clients to send reactions using a key that conveys a similar meaning to the image? In that way, clients that use the server-side aggregations will (as mentioned above) fall back to displaying only the key, so they won't be affected by collisions since they won't display the images. And clients that don't use server-side aggregations will see the full events, and be able to distinguish the reactions based on the url field.