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.

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

MSC3382: Inline message Attachments #3382

Open
wants to merge 40 commits into
base: main
Choose a base branch
from

Conversation

MurzNN
Copy link
Contributor

@MurzNN MurzNN commented Sep 7, 2021

@MurzNN MurzNN changed the title MSCXXXX: Inline message Attachments MSC3382: Inline message Attachments Sep 7, 2021
MurzNN added a commit to MurzNN/matrix-doc that referenced this pull request Sep 7, 2021
Option two is moved to matrix-org#3382
Also added link to matrix-org#3051 with array implementation of relations.
@turt2live turt2live added client-server Client-Server API kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal proposal-in-review labels Sep 7, 2021
Copy link
Contributor

@kevincox kevincox left a comment

Choose a reason for hiding this comment

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

Option 1 looks good.


*This MSC is especially for media image attachments to message, but I try to make it extendable for multiple attachment types (files, videos, and in future - external URLs, links to other Matrix events, etc). So, in most of examples, I am using "image", but it means, that, instead of image, there may be another attachment type.*

In the current implementation each media (file, image, video) can be sent only via a separate event to the room. But in most cases even one media is not sent alone, it must be commented via some text by the sender. So the user often wants to attach some images (one or several) directly to their text message when composing it, not after or before it.
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 the current implementation each media (file, image, video) can be sent only via a separate event to the room. But in most cases even one media is not sent alone, it must be commented via some text by the sender. So the user often wants to attach some images (one or several) directly to their text message when composing it, not after or before it.
In the current implementation each media (file, image, video) can be sent only via a separate event to the room. But in most cases media items are not sent alone, it is accompanied by some description by the sender. The user often wants to attach some images (one or several) directly to their text message when composing it, not after or before it.


In the current implementation each media (file, image, video) can be sent only via a separate event to the room. But in most cases even one media is not sent alone, it must be commented via some text by the sender. So the user often wants to attach some images (one or several) directly to their text message when composing it, not after or before it.

And now the user can send images only before the message (or after it) as a separate message, but they can't attach images during the composing process to send them when the text is finished, together with the text message in one event.
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
And now the user can send images only before the message (or after it) as a separate message, but they can't attach images during the composing process to send them when the text is finished, together with the text message in one event.
Currently the user can send images only before the message (or after it) as a separate message, but they can't attach images during the composing process to send them when the text is finished, together with the text message in one event.


And now the user can send images only before the message (or after it) as a separate message, but they can't attach images during the composing process to send them when the text is finished, together with the text message in one event.

On the display side, when the user sends multiple images, the problem is that each image is displayed alone, as separate event with full width in timeline, not linked to the message, and not grouped to the gallery.
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
On the display side, when the user sends multiple images, the problem is that each image is displayed alone, as separate event with full width in timeline, not linked to the message, and not grouped to the gallery.
On the display side, when the user sends multiple images each image is displayed alone, as separate event with full width in timeline, not linked to the message, and not grouped to the gallery.

Nit: superfluous.


On the display side, when the user sends multiple images, the problem is that each image is displayed alone, as separate event with full width in timeline, not linked to the message, and not grouped to the gallery.

Messages with multiple attachments now already implemented in many messengers, for example - in Skype, Slack, VK Messenger. And Matrix, because lack of support, now have problems with bridging those messages to Matrix rooms.
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
Messages with multiple attachments now already implemented in many messengers, for example - in Skype, Slack, VK Messenger. And Matrix, because lack of support, now have problems with bridging those messages to Matrix rooms.
Messages with multiple attachments now already implemented in many messengers, for example - in Skype, Slack, VK Messenger. Matrix has problems bridging to these services because multiple attachments, and attachments with descriptions are not supported.


When user press "Send" button, Matrix client do the upload of all media, that user attached to message, as separate events to room (how it is done now), before sending message with typed text. And after sending of all attachments is finished, client send message with aggregating event, using `m.relates_to` field (from the [MSC2674: Event relationships](https://github.com/matrix-org/matrix-doc/pull/2674)), that points to all previously sent events with media, to group them into one gallery.

For exclude showing those events in modern clients before grouping event added, I propose extend separate media events via adding "marker" field `is_attachment: true`, if clients got this value - they must exclude showing this media in timeline, and shows them only in gallery with grouping event.
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
For exclude showing those events in modern clients before grouping event added, I propose extend separate media events via adding "marker" field `is_attachment: true`, if clients got this value - they must exclude showing this media in timeline, and shows them only in gallery with grouping event.
Clients that support this MSC should hide events with `is_attachment:true` and wait until the aggregating event is received. Once the aggregating event is received all events should be shown together, for example using a gallery for multiple images.

Nit: is_attachment was already mentioned so I didn't feel it needed to be introduced again. I also feel that the new description is much simpler.

]
```

For delete (redact action) message with attachments, we must also apply `redact` action to each message attachment event too.
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
For delete (redact action) message with attachments, we must also apply `redact` action to each message attachment event too.
When redacting messages with attachments, each message referenced by `m.attachment` should be redacted as well.

Fixed wording to use the matrix feature name. I also downgraded this to "should" because I can imagine cases where the user wanted to redact just the message, but leave the media. I think redacting everything should be the "default" but I will leave the UX up to the clients.


#### Fallback:

I see no serious problems with fallback display of attachments. For Matrix clients, that don't yet support this feature, the attachments will be represented as separate media events, like the user upload each attachment separately, before sending main message.
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
I see no serious problems with fallback display of attachments. For Matrix clients, that don't yet support this feature, the attachments will be represented as separate media events, like the user upload each attachment separately, before sending main message.
I see no serious problems with fallback display of attachments. For Matrix clients, that don't yet support this feature, the attachments will be represented as separate media events, like the user upload each attachment separately, before sending the main message.

When user press "Send" button, Matrix client do the upload of all media, that user attached to message, as separate events to room (how it is done now), before sending message with typed text. And after sending of all attachments is finished, client send message with aggregating event, using `m.relates_to` field (from the [MSC2674: Event relationships](https://github.com/matrix-org/matrix-doc/pull/2674)), that points to all previously sent events with media, to group them into one gallery.

For exclude showing those events in modern clients before grouping event added, I propose extend separate media events via adding "marker" field `is_attachment: true`, if clients got this value - they must exclude showing this media in timeline, and shows them only in gallery with grouping event.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if the aggregating event is never received? I think that we should recommend clients show the "attachments" after some timeout.


If [MSC2398: proposal to allow mxc:// in the "a" tag within messages](https://github.com/matrix-org/matrix-doc/pull/2398) will be merged before this, we can replace `http` urls to direct `mxc://` urls, for support servers, that don't allow downloads without authentication and have other restrictions.

**If we will come up with better fallback display, maybe bring this option as main suggestion?**
Copy link
Contributor

Choose a reason for hiding this comment

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

I like that you can reply to individual media elements in the previous version.

Also in general I think that more small events is better than big events. That being said neither of these should have to be particularly scalable.

@@ -0,0 +1,198 @@
# MSCXXXX: Inline message attachments
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks very similar to option 2?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants