Responsive Images: Don't add srcset attributes to animated gifs.

[joemcgill]

When an animated gif is uploaded to WordPress, the resized versions lose animation (see #28474) so we shouldn't add a srcset attribute to gifs when the full size image is used.

[kirasong]

I was chatting with @joehoyle about this. Two things:

• Since we're already flattening any resized image, this should only be a problem if the original size is being specifically chosen, correct? Maybe we can specifically target those, to make responsive images for for the other cases?

• PNGs can also be animated, so we need to address them as well, and I would hate to see them lose responsive support completely. If we do the above, it should keep most of the support intact.

[joemcgill]

Replying to DH-Shredder:

I was chatting with @joehoyle about this. Two things:

• Since we're already flattening any resized image, this should only be a problem if the original size is being specifically chosen, correct? Maybe we can specifically target those, to make responsive images for for the other cases?

Totally agree. My patch, 34528.diff​ does specifically target cases where the original size is chosen.

• PNGs can also be animated, so we need to address them as well, and I would hate to see them lose responsive support completely. If we do the above, it should keep most of the support intact.

Are a lot of people using animated PNGs? I worry about doing the same for PNGs because often times I see people inserting full size PNGs into their content that are essentially screenshots with very large file sizes. I'd hate to lose srcset support for these cases.

[kirasong]

Replying to joemcgill:

My patch, 34528.diff​ does specifically target cases where the original size is chosen.

Okay, cool! Sorry about that; nice.

Are a lot of people using animated PNGs? I worry about doing the same for PNGs because often times I see people inserting full size PNGs into their content that are essentially screenshots with very large file sizes. I'd hate to lose srcset support for these cases.

I don't suspect there are many, but want to make sure that we don't break user expectations, since this would change displayed user content out from under them.

We may want to (for GIFs/PNGs) exclude the original fromsrcset if the src= does not reference the original, so that some sizes that were not animated before don't magically become animated after the update.

[azaozz]

In 35524:

Responsive images: do not generate srcset for GIFs that are inserted at full size. Prevents breaking animated GIFs.

Props joemcgill.
Fixes #34528.

[kirasong]

Opening this back up for discussion on PNGs and whether we should add bits to keep non-animated PNGs or GIFs from becoming animated after update.

[kirasong]

I chatted with @joemcgill, and he pointed this out: ​http://caniuse.com/#feat=apng

Basically, with only 18% global support across browsers with APNGs, it's very unlikely that it's being used heavily, and he's seen regular use of "original" size for PNGs specifically that would be helped by supporting RespImg.

He notes that it might be a good idea to mark animated images when inserted in meta. It might help to do this before/at the same time as we fix #28474, so we know for sure when it's safe to use RespImg on GIFs.

I'd like to use this ticket to not include the full size animated GIF when we do generate srcset, so that all of the cases are covered with GIFs.

[joemcgill]

In 34528.2.diff​, full size images are omitted from srcset attributes when the original file is an intermediate sized GIF so we don't accidentally add animation to an otherwise flat image. Tests are updated as well.

[kirasong]

Adjusted comments

[kirasong]

Thanks @joemcgill -- this looks good.

Clarified the reason for the adjustment (WordPress flattening GIFs) and normalized 'srcset' to srcset in 34528.3.diff​.

[joemcgill]

34528.4.diff​ includes the docs changes from 34528.3.diff​ and streamlines the logic that checks for GIFs in wp_cacluclate_image_srcset() by flipping the conditionals.

[kirasong]

34528.4.diff​ Looks good. Passes tests.

[SergeyBiryukov]

34528.5.diff​ makes the condition a bit more readable. Tests still pass :)

[azaozz]

In 35561:

Responsive images: omit full size images from srcset attributes when the original file is an intermediate sized GIF so we don't accidentally add animation to an otherwise flat image. Update the tests to cover this case.

Props joemcgill, H-Shredder, SergeyBiryukov.
Fixes #34528.

[keketovic]

Has this been fixed? Still hapenning in the latest wordpress.

[DrewAPicture]

@keketovic This should be fixed in trunk, e.g. 4.5 Beta 2.

[joemcgill]

Replying to keketovic:

Has this been fixed? Still hapenning in the latest wordpress.

Hi @keketovic,

Can you share an example of where you're seeing the issue? Responsive images shouldn't cause your animated GIFs to break in WP 4.4. Note that only the full size version of a GIF will be animated, and any other sizes (thumbnail, medium, etc.) will not be animated. This has been the behavior of WordPress quite some time (see #28474).

[Alignak]

I think, excluding gifs from srcsets should be optional.
I mean, how hard can it be to add another checkbox for those that want to generate srcsets for gifs?