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

MSC3746: Render image data in reactions #3746

Closed
wants to merge 2 commits into from
Closed

MSC3746: Render image data in reactions #3746

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5295021
get a msc number
AndrewRyanChama Feb 25, 2022
02b9436
Update 3746-image-reactions.md
AndrewRyanChama Feb 25, 2022
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
77 changes: 77 additions & 0 deletions proposals/3746-image-reactions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# MSC3746: Render Images in Reactions

Many messaging services such as Slack and Discord allow for custom images for reacts, rather than only standard emoji. Currently Element will only show the plain text of the reaction key. In order to achieve parity with other messaging services and satisfy customer needs, we must implement a way to have reactions which can be any image, rather than one of the unicode emojis.
Copy link
Member

Choose a reason for hiding this comment

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

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


This proposal is concerned only with providing an event format which can describe image reactions. Any way for users to compose and send these reactions such as pickers and image packs is out of scope.

## Proposal

Currently, reactions are implemented as room events which have a relationship with the event they are reacting to. Identical reactions by different users are grouped together by the reaction key, and the reaction key (text/emoji) is displayed under the related message.

An example of a reaction, which displays 😀
```json
{
"type": "m.reaction",
"sender": "@testme:localhost:8008",
"content": {
"m.relates_to": {
"rel_type": "m.annotation",
"event_id": "$hPRRsFL03pNRE2tnmzzqyP6OXofz-bbsrQbRWJDA4p0",
"key": "😀"
}
},
"origin_server_ts": 1645830754708,
"unsigned": {
"age": 28,
"transaction_id": "m1645830754585.0"
},
"event_id": "$8dZO0nnIoz-uWyDfevNcaGcg5p6e7oK7CoKOwe-aWTM",
"room_id": "!uLFjhtpHuebWoCiyvz:localhost:8008"
}
```

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

Choose a reason for hiding this comment

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

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.


content of an image reaction:
```json
"content": {
"m.relates_to": {
"rel_type": "m.annotation",
"event_id": "$hPRRsFL03pNRE2tnmzzqyP6OXofz-bbsrQbRWJDA4p0",
"key": "😀"
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@HarHarLinks HarHarLinks Sep 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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

},
"url": "mxc://matrix.org/VOczFYqjdGaUKNwkKsTjDwUa",
"info": {
Comment on lines +43 to +44
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

"w": 256,
"h": 256,
"size": 214537,
"mimetype": "image/png",
"thumbnail_url": "mxc://matrix.org/VOczFYqjdGaUKNwkKsTjDwUa",
"thumbnail_info": {
"w": 256,
"h": 256,
"size": 214537,
"mimetype": "image/png"
}
}
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

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.


### Server side aggregation
Server side aggregation will remain unchanged, where only the keys and counts of those keys are aggregated. In the case where the client is unable to use client-side aggregation and must display reactions based on the server-side aggregation, then it will fallback to displaying only the key for reactions.

Element web and desktop do not make use of server-side aggregation for reactions, so they are currently unaffected by this.

### Fallback
Older clients will simply display the reaction emoji or plaintext. If an older client also clicks the reaction, it will send a reaction event without the image content. Other clients must check all aggregated events to find which ones include the image. If all newer clients unreact, then the image will be lost and the reaction will revert to plaintext/emoji.

## Potential issues

It's possible that different users will send reactions with different images under the same reaction key, either due to malicious action or collisions. Reaction senders must take this into account and use a key that will not collide with previously existing reactions.
Copy link
Member

Choose a reason for hiding this comment

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

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.


This would remove the chance of mismatches between key and image, but would not give an experience to older clients.

## Alternatives

Instead of including the image information in the event content, we could include everything necessary in the relation key. For example the key could be an mxc url, json, markdown, or reference to an external data source (such as an image set-as-room).