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

MSC2385: Disable URL Previews, alternative method #2385

Open
wants to merge 2 commits into
base: master
from

Conversation

@Sorunome
Copy link
Contributor

Sorunome commented Dec 8, 2019

Rendered

This MSC provides an alternative method to MSC2376

Signed-off-by: Sorunome mail@sorunome.de

@bmarty

This comment has been minimized.

Copy link

bmarty commented Dec 9, 2019

Other potential issue: naughty clients could add some urls in url_previews that are not in the message, resulting in URL preview displaying bad content not related to the message.
Why not having just a list of booleans to avoid repeating the url in the message?

@KitsuneRal

This comment has been minimized.

Copy link
Member

KitsuneRal commented Dec 9, 2019

Other potential issue: naughty clients could add some urls in url_previews that are not in the message, resulting in URL preview displaying bad content not related to the message.
Why not having just a list of booleans to avoid repeating the url in the message?

Further on, please comment on a specific line of text so that discussions could be tracked more easily. As per your concern - as far as I see, the MSC states that URLs in url_previews must be exactly the URLs in the message body, so a boolean would be superfluous.

Copy link
Member

KitsuneRal left a comment

Barring a few wording fixes, I really like this approach.

proposals/2385-no-url-previews.md Outdated Show resolved Hide resolved
proposals/2385-no-url-previews.md Outdated Show resolved Hide resolved
proposals/2385-no-url-previews.md Outdated Show resolved Hide resolved
proposals/2385-no-url-previews.md Outdated Show resolved Hide resolved
@turt2live turt2live self-requested a review Dec 9, 2019
proposals/2385-no-url-previews.md Outdated Show resolved Hide resolved
@Sorunome

This comment has been minimized.

Copy link
Contributor Author

Sorunome commented Dec 9, 2019

Other potential issue: naughty clients could add some urls in url_previews that are not in the message, resulting in URL preview displaying bad content not related to the message.
Why not having just a list of booleans to avoid repeating the url in the message?

As stated in the MSC, url_previews is basically a whitelist, not a list of URLs to do previews for. So you still do URL previews like currently but additionally check if it is present in url_previews (if present). Thus additional URLs in that list would be just ignored.

@dali99

This comment has been minimized.

Copy link

dali99 commented Dec 9, 2019

Honestly I kinda like the potential to hint clients to render "previews" for links not present in the message.

I believe facebook does this, where you can paste in a link, get a preview, remove the link and then just comment on the link being shared.

essentially turning the preview into more of a "rich link" part of the message.

@bmarty

This comment has been minimized.

Copy link

bmarty commented Dec 10, 2019

the MSC states that URLs in url_previews must be exactly the URLs in the message body

Yes, but what if it's not the case?

Anyway, maybe the pb is to avoid rendering useless url preview in the timeline, so maybe the solution is to stop displaying url preview in the timeline and display the preview in a popup when the link is hovered or when the link is clicked (the same way Google docs do it for instance).

@Sorunome

This comment has been minimized.

Copy link
Contributor Author

Sorunome commented Dec 10, 2019

the MSC states that URLs in url_previews must be exactly the URLs in the message body

Yes, but what if it's not the case?

It is supposed to be ignored, as stated in the MSC

If a URL is present in this key that is not present in the body clients SHOULD NOT render a preview for that URL.

@KitsuneRal

This comment has been minimized.

Copy link
Member

KitsuneRal commented Dec 10, 2019

Honestly I kinda like the potential to hint clients to render "previews" for links not present in the message.

I believe facebook does this, where you can paste in a link, get a preview, remove the link and then just comment on the link being shared.

essentially turning the preview into more of a "rich link" part of the message.

I would absolutely prefer such behaviour to be in a separate MSC. Matrix operates in a very different realm than Facebook; I could imagine such behaviour as somewhat natural in Mastodon but not in Matrix.

@dali99

This comment has been minimized.

Copy link

dali99 commented Dec 10, 2019

Honestly I kinda like the potential to hint clients to render "previews" for links not present in the message.
I believe facebook does this, where you can paste in a link, get a preview, remove the link and then just comment on the link being shared.
essentially turning the preview into more of a "rich link" part of the message.

I would absolutely prefer such behaviour to be in a separate MSC. Matrix operates in a very different realm than Facebook; I could imagine such behaviour as somewhat natural in Mastodon but not in Matrix.

rich-links

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.