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

Jump to bottom

MSC3286: Media spoilers #3286

Closed
wants to merge 4 commits into from
Closed

MSC3286: Media spoilers #3286

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
684c86f
Add proposal for media spoilers
robintown Jul 18, 2021
b9b2e05
Remove stray trailing commas
robintown Jul 18, 2021
c3abdec
Allow tagging audio and file messages as spoilers
robintown Jul 19, 2021
c05b1df
Consistently say 'messages', not 'events'
robintown Jul 20, 2021
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
80 changes: 80 additions & 0 deletions proposals/3286-media-spoilers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# MSC3286: Media spoilers
Copy link

Choose a reason for hiding this comment

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

I actually feel like this could be made a lot more generic. There are various reasons why one may choose to hide something by default including: spoilers, nsfw content, graphic content or medical reasons (like epilepsy). For the most part, the client wouldn't need to tell a difference, but in some cases (like NSFW), it would be useful for the client to know as it could then have a global setting to permanently hide such posts. The lack of this has also been the reason for implementing a basic description-based filtering in Element Android, but that could be replaced with this.

I would suggest spoiler be replaced with something like hidden or hidden_by_default with the subkeys reason and description, the former being machine-readable (m.nsfw, m.spoiler, m.graphic, m.medical for instance) and latter, human-readable written by the sender (like, can cause epileptic seisures).

If this is too generic or out-of-scope for this MSC, let me know, but I figured it wouldn't hurt to suggest.


[MSC2010](2010-spoilers.md) created ways for clients to tag parts of messages as
spoilers, enabling receiving clients to hide spoilered content unless the user
explicitly chooses to view it. While the proposal only covered textual spoilers,
it is also often desirable to be able to mark images and videos as spoilers,
since many clients automatically display thumbnails for media that recipients
might not want to see.

As with textual spoilers, there are a variety of reasons that one might want to
mark an image or video as a spoiler, for example if it would spoil a story, or
if some recipients might find the content objectionable or upsetting to
accidentally see. By marking such media as a spoiler, clients can then take
measures to require consent from the user before displaying the content.

## Proposal

To support this, an optional `spoiler` property of type `string` is added to the
`content.info` dictionaries of `m.room.message` events with a `msgtype` of
`m.image` or `m.video`. When present, it indicates that the given media has been
robintown marked this conversation as resolved.
Show resolved Hide resolved
tagged as a spoiler.

The value of `spoiler` represents a placeholder text that clients may display as
the reason for the spoiler. Providing a reason is optional, and one may indicate
the absence of a reason by setting `spoiler` to an empty string.

### Examples

Content of an image with a spoiler reason:

```json
{
"body": "screenshot.png",
"info": {
"mimetype": "image/png",
"size": 123456,
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be nice to require or at least suggest, that clients should send a blurhash for spoilers (of image like files), that the client can display in place of the actual image. Might make not much sense to include in the MSC, since #2448 is still an MSC too, but it would probably make the spoiler experience much nicer!

Copy link
Member Author

Choose a reason for hiding this comment

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

There are plenty of ways clients might render spoilers without using blurhashes, so I don't think it makes sense to require them. Plus, if a client can render blurhashes and wants to use them for media spoilers, it's very likely to already have a local blurhash encoding implementation that it can use anyways, so I think blurhashes deserve a tangential mention here at most.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to be able to always show a blurhash, but I guess you can't anyway, so alright, makes sense!

Copy link
Contributor

Choose a reason for hiding this comment

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

There are plenty of ways clients might render spoilers without using blurhashes, so I don't think it makes sense to require them. Plus, if a client can render blurhashes and wants to use them for media spoilers, it's very likely to already have a local blurhash encoding implementation that it can use anyways, so I think blurhashes deserve a tangential mention here at most.

While there are other ways I think a blurhash would be save more resources here. So the Client doesnt need to download the thumbnail files until a User interaction happened.

"spoiler": "Contains spoilers for chapter 6"
},
"msgtype": "m.image",
"url": "mxc://example.org/abcdef"
}
```

Content of a video without a spoiler reason:

```json
{
"body": "recording.mp4",
"info": {
"mimetype": "video/mp4",
"size": 123456,
"spoiler": ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you thought of doing "spoiler": true for spoilers without a reason? That might choke on some languages due to the spoiler attribute being potentially a string or a boolean.

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer this method.

  1. Easier for statically typed languages to manage.
  2. "" is a natural value for "no reason" since the reason is empty.
  3. If spoiler: true is allowed then you still need to support spoiler: "" unless we disallow it. However disallowing it adds complexity on the clients because now they have to map no user reason to true, mapping to "" probably happens natural in most implementations..
  4. It raises questions about what spoiler: false means. (Presumably it means the same as null or missing, but now we have an extra representation for the same information).

Copy link
Contributor

@HarHarLinks HarHarLinks Feb 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

By that argument, I would agree types shouldn't get mixed, but the natural way to express "no reason" would be... to not supply this optional field (which is equal to setting null). A reason of "" makes little sense to have, I would consider it bad style by the sending client and leave it to the receiving client whether to handle it specially or show an empty string.

Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't null represent "not a spoiler" but "" represent "is a spoiler, but no rationale provided"?

Maybe a better schema is making this an object with a reason_text attribute and maybe can have more structured attributes in the future (translations, structured info on what the spoiler is spoiling...). That way {} is a spoiler is no rationale provided and {reason: "Harry Potter 4 ending"} is a spoiler with rationale. And in the future we could have {subjects: ["Harry Potter 3"]} for a spoiler about a well specified topic but no rationale (maybe I have seen it so can tell my client to always reveal thins about this show).

Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't null represent "not a spoiler" but "" represent "is a spoiler, but no rationale provided"?

Sorry, you're right.

better schema is making this an object

I think that is what I really had in mind, so I would agree. In short, empty string feels like a hack when we could use proper structured data, even if the only way to use it remains the reason field.

Copy link
Contributor

Choose a reason for hiding this comment

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

#3725 does pretty much this :)

},
"msgtype": "m.video",
"url": "mxc://example.org/abcdef"
}
```

## Potential issues

None that the author is aware of.

## Alternatives

An alternative solution, which some people currently use, is to embed images
inline in `m.text` events, and then tag it using the existing mechanism for
textual spoilers. However, this is a rather hacky workaround, as it does not
support videos, nor does it support encryption. It also loses the semantics of
standalone `m.image` events, which makes it difficult for clients to render
image spoilers differently from regular textual spoilers.

## Security considerations

None that the author is aware of.

## Unstable prefix

Clients wishing to experimentally implement this proposal may do so by replacing
the `spoiler` key in `m.image` and `m.video` events with
`town.robin.msc3286.spoiler`.