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

MSC4230: Flag for animated images #4230

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

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Nov 21, 2024

Proposes adding a boolean flag to images that indicates whether they're animated.

Impl: element-hq/element-web#28513

Rendered

Disclosure: This is presented wearing my, "Element Employee and Element Web Engineer" hat.

@turt2live turt2live 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 Nov 21, 2024
@turt2live turt2live marked this pull request as draft November 21, 2024 16:24
@dbkr dbkr marked this pull request as ready for review November 21, 2024 17:07
@dbkr dbkr changed the title [Placeholder] Flag for animated images Flag for animated images Nov 21, 2024
@dbkr dbkr changed the title Flag for animated images [MSC4230]: Flag for animated images Nov 21, 2024
@dbkr dbkr changed the title [MSC4230]: Flag for animated images MSC4230: Flag for animated images Nov 21, 2024

## Proposal

We add an optional boolean flag, `ia_animated` to the `info` object of image events indicating if
Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, this field would be allowed in the info on msgtype: m.image and type: m.sticker events?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess just images for now: clarified.

Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to allow it for stickers?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not really sure to be honest, which is exactly why I'm leaving it for someone else to propose if they think it's appropriate.

Copy link
Contributor

Choose a reason for hiding this comment

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

Given that stickers are ~renamed images, I agree with Sumner that the same should apply to stickers.

We don't have extensible events yet, but MSC3552 goes in the same direction about the images and stickers relation ship.

Co-authored-by: R Midhun Suresh <[email protected]>

Clients may lie about the flag which would cause unexpected behaviour, for example, an image which
was not presented might then animate when the user clicks to enlarge it, allowing for 'jump scares'
or similar. Clients may wish to prevent images from being animated if the flag is set to false.
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't be sensible for an implementation to use this flag only as a hint as to whether it should preload the original, and probe the media regardless of when it was downloaded to confirm the animation? I feel like this MSC is recommending a client behaviour that is a tad too "trusting".

Copy link
Member Author

Choose a reason for hiding this comment

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

They could do that if they saw a need to. I've tried to be specific though and call out a distinct case where clients might want to take care over. I don't think it's really the spec's job to enumerate every detail that clients might possibly mess up, although we can certainly give more hints if you think there's something else worth calling out.


## Proposal

We add an optional boolean flag, `is_animated` to the `info` object of `m.image` events indicating if
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to bring it up, we could also give this flag a more general name prefetch_original to express "you might wanna prefetch this", and apply it optionally to any attachment type. A client might assume based on the attachment type, mime type, flag, and other metadata combinations, whether it wants to prefetch, e.g. to play an animated image/gif without delay. Then again I fail to come up with other reasons you might want to hints about prefetching attachments.

Copy link
Member Author

Choose a reason for hiding this comment

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

Personally I'm more persuaded by the idea of giving information about the media rather than trying to second guess what the client might want to do.

Co-authored-by: Will Hunt <[email protected]>
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.

6 participants