-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Fix some webp images improperly marked as animated #29713
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
Fix some webp images improperly marked as animated #29713
Conversation
This looks sane, good spot on the byte mistake, but would really want a test. Seriousness of the image matters not, copyright-esque issue however do. Keep in mind the CLA you signed for anything you contribute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well that got a good laugh from me
The added test is pretty minimal, should I add a check that the new file does indeed contain the |
@Petersmit27 I trust that you checked it, that's enough |
Plus in theory coverage will tell us |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this
* Fix some webp images improperly marked as animated * Add unit test for an unanimated webp file in extended file format * Apply linting to webp test
Sorry about the flakes above, they're unrelated to your change and yet to be investigated by the maintainer |
Going to wait while @florianduros looks into the flakes |
Depending on the webp file, Element incorrectly detects a webp as animated because the wrong byte is looked at.
If GIF autoplay is disabled, it leads to WEBP files getting the "GIF" banner while they are not actually animated.
Before
After
What was wrong
According to the webp spec referenced in the code, the byte highlighted in the image below is checked, but it should be one in the next row (4 bytes later).

Why the unit test passed before
The
Static WEBP
test inblobIsAnimated
inImage-test.ts
passed before this change because the tested file is in theVP8L
format, which indeed cannot be animated.The webp file I have is in the
VP8X
(extended file) format, which may be animated, but in this case is not.To make the test fail, you can use it (GitHub doesn't let me directly upload webp files) in place of the current
test/unit-tests/images/static-logo.webp
.If needed, I could replace the current
static-logo.webp
with the one I have for this PR, though maybe you would prefer to have a more serious image for that.Checklist
public
/exported
symbols have accurate TSDoc documentation.