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

MSC3971: Sharing image packs #3971

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

Conversation

AndrewRyanChama
Copy link

@AndrewRyanChama AndrewRyanChama commented Feb 25, 2023

Rendered
Signed-off-by: Andrew Ryan andrewryanchama@gmail.com
author: @AndrewRyanChama

@AndrewRyanChama AndrewRyanChama changed the title Sharing image packs MSC3971: Sharing image packs Feb 25, 2023
@uhoreg uhoreg added proposal A matrix spec change proposal 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. labels Feb 25, 2023

With the following requirements:

1. Links work inside and outside Matrix clients.
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the use case for having this work outside of Matrix clients?

Copy link
Author

Choose a reason for hiding this comment

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

The links should be sharable outside of Matrix, such as over other social media. This can allow users to post lists and collections of image packs on those platforms.

I'll update the doc to say that links should be sharable outside of Matrix clients - the link itself will still lead to opening Matrix.

Copy link
Contributor

Choose a reason for hiding this comment

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

That means I should be able to share a link outside matrix, but I don't need the embedding in the message to be accessible outside matrix, right? I.e. it doesn't need to be a link embedded into the img tag for that to work, but a data attribute fulfills that in the same way, since the attribute would still be a matrix.to link?

This means you probably want to add some metadata to the matrix.to link, so that it can be distinguished from a normal event link, I guess? I.e. appending a ?image_pack or so?

Copy link
Author

Choose a reason for hiding this comment

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

Right, basically I just mean we want a matrix.to link, rather than some bespoke link that can only be clicked within a matrix client. We don't need to expose any new data to the outside world.

Copy link
Author

Choose a reason for hiding this comment

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

Adding a query param won't automatically be passed through matrix.to to the matrix client. This might be a useful expansion, but it adds a lot of complexity and I think it's functional as-is.

The person sharing the matrix.to link should be responsible for describing what it is - already a matrix.to event link doesn't give the room name or any other info, so sharers already have to do this.

Copy link
Contributor

Choose a reason for hiding this comment

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

You probably want clients as well as intermediates (like the matrix.to page) to know what the link is for before or without resolving it to the actual event. Otherwise matrix.to will just show "you were invited to room X". You probably want to be able to show, that you were linked to a sticker pack instead, which needs additional metadata in the link. Similarly you probably want clients to suggest you to add the sticker pack instead of scrolling to the event. This means again, you want extra metadata. In general such parameters are passed to clients, that can handle matrix.to, but possibly it would need to be supported by the matrix: scheme as well.

You don't really want clients to have to do some deep inspection of every matrix.to target to be able to use it and you don't want to have users paste the link into some field, but in the best case have it be automatic when clicking a link, that the client does the right thing. It is less complex to just add metadata to the matrix.to link than have every client do deep inspection of the link target before prompting the user what to do. That is why matrix.to already distinguishes between room, user and event links and why the matrix URI format has a specific action parameter, describing the purpose of a link. This is handled before any request is sent to a server, so that the user can decide what to do.

Copy link
Author

Choose a reason for hiding this comment

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

Hmmm, action= seems like the right thing to do here. Unfortunately, matrix.to and also Element don't honor this value. You can see here that https://github.com/matrix-org/matrix.to/blob/main/src/open/clients/Nheko.js#L49 that action is harcoded depending on the resource type. I think the only query params that work here are the ones on the matrix.to homepage. Matrix.to always shows the same prompt "choose app to continue" regardless of the room type.

I think adding an additional metadata to the url does make sense, but it needs a bunch of additional things to move at the same time. I'll document a bit more about what is wanted here though.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can just add an unrecognized action. Clients that want to use that metadata can then do that, but if they don't recognize it, they should just ignore it and handle it like normal events, which I would say is fine :)

create a proposal for sharing image packs.
@deepbluev7
Copy link
Contributor

(Small note, as far as I am aware, force pushes are generally frowned upon for MSCs, because it makes it harder to review changes to the MSC. Afaik it is suggested to just push new commits and they are squashed, once the MSC is merged)

@AndrewRyanChama
Copy link
Author

Dropping my take on the implementation here: SchildiChat/matrix-react-sdk#15

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.

None yet

3 participants