element-hq / element-web

A glossy Matrix collaboration client for the web.
https://element.io
GNU Affero General Public License v3.0
11.25k stars 2.01k forks source link

WebP/APNG images shown as GIF #24015

Open germain-gg opened 1 year ago

germain-gg commented 1 year ago

Steps to reproduce

  1. Send a webp image. See the GIF

Outcome

What did you expect?

Not to see the "GIF" toast on the top left

What happened instead?

Screenshot 2022-12-15 at 09 37 27

Operating system

No response

Browser information

No response

URL for webapp

No response

Application version

No response

Homeserver

No response

Will you send logs?

No

t3chguy commented 1 year ago

Yeah IIRC some other platform does the same thing so I copied it - thought "WEBP" would just look confusing and some platforms even send "GIF"s as MP4s nowadays so the word seems to have lost all meaning.

germain-gg commented 1 year ago

I don't really understand why an overlay is needed here. We do not show anything for JPG or PNG. Why treat webp differently?

I can understand the argument for animated webp. But the images above are still.

Not quite sure why you added the X-Needs-Design label for? This sounds more like a defect to me where we should not show the GIF overlay at all.

t3chguy commented 1 year ago

It should be only shown on animated WEBP images. The overlay is there as a hint that hovering plays the animation. blobIsAnimated actually checks for the animated flag in the blob.

It even has tests - maybe your image is actually a 1-frame animated WEBP image

image

Indeed, the label was assuming that you were complaining it said GIF for an animated WEBP. Nothing in the OP made it clear they weren't animated, screenshots are poor at conveying animation.

HarHarLinks commented 1 year ago

maybe your image is actually a 1-frame animated WEBP image

could Element be made to check for that? I have had the same issue before. There are always going to be "improper" webps like this in the wild as this isn't usually obvious to users.

t3chguy commented 1 year ago

Yes it could, but writing a full WEBP parser would add a lot more complexity and edge cases to test. I couldn't find a minimal one to just import at the time of writing it initially. Contributions welcome.