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

Image is partially converted to WebP #69

Closed
Tracked by #22
mitogh opened this issue Dec 22, 2021 · 7 comments
Closed
Tracked by #22

Image is partially converted to WebP #69

mitogh opened this issue Dec 22, 2021 · 7 comments
Labels
[Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) [Type] Bug An existing feature is broken

Comments

@mitogh
Copy link
Member

mitogh commented Dec 22, 2021

Note

Make sure the Performance plugin is enabled and activated, as well with the webp modules for images at: Settings > Performance

Related with issue:

Steps to recreate

  1. Open the WordPress dashboard.
  2. Go to Media > Add New
  3. Upload the image
  4. Visit the attachment page of the image.

2021-12-22_16-08

Generated Markup:

<img 
width="1920" 
height="2400" 
src="http://galaxy.lndo.site/wp-content/uploads/2021/12/sami-takarautio-NEHr44foE54-unsplash.jpg" 
class="attachment-full size-full" 
alt="" loading="lazy" 
srcset="http://galaxy.lndo.site/wp-content/uploads/2021/12/sami-takarautio-NEHr44foE54-unsplash.jpg 1920w,
http://galaxy.lndo.site/wp-content/uploads/2021/12/sami-takarautio-NEHr44foE54-unsplash-240x300.webp 240w, 
http://galaxy.lndo.site/wp-content/uploads/2021/12/sami-takarautio-NEHr44foE54-unsplash-819x1024.webp 819w, 
http://galaxy.lndo.site/wp-content/uploads/2021/12/sami-takarautio-NEHr44foE54-unsplash-768x960.webp 768w, 
http://galaxy.lndo.site/wp-content/uploads/2021/12/sami-takarautio-NEHr44foE54-unsplash-1229x1536.webp 1229w, 
http://galaxy.lndo.site/wp-content/uploads/2021/12/sami-takarautio-NEHr44foE54-unsplash-1638x2048.webp 1638w, 
http://galaxy.lndo.site/wp-content/uploads/2021/12/sami-takarautio-NEHr44foE54-unsplash-1568x1960.webp 1568w" 
sizes="(max-width: 1920px) 100vw, 1920px" 
style="width:100%;height:125%;max-width:1920px;"
>

Original image to replicate the issue.

sami-takarautio-NEHr44foE54-unsplash

@felixarntz felixarntz added [Focus] Images [Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) [Type] Bug An existing feature is broken labels Dec 22, 2021
@felixarntz felixarntz added this to To do in [Focus] Images Dec 22, 2021
@felixarntz felixarntz moved this from To do to Triage in [Focus] Images Dec 22, 2021
@felixarntz
Copy link
Member

@mitogh This may be expected, as the uploaded source image is not being converted to WebP. cc @adamsilverstein

@adamsilverstein
Copy link
Member

This is expected because we keep the original image (always). I'd love to test this further though to see how older Safari (MacOS), or even outlook or ie handle this img tag markup - to they fall back to the jpeg since they don't support webp? If so this helps alleviate the immediate need for the picture element.

@adamsilverstein
Copy link
Member

Closing as "works for me" since this is the expected behavior. Lovely image though!

[Focus] Images automation moved this from Backlog to Done Jan 18, 2022
@mitogh
Copy link
Member Author

mitogh commented Jan 18, 2022

While running some tests I face a new test case that would be worth considering how we are going to handle:

  1. Uploading a quite large image (higher than the default supported dimension from WordPress 2500px) (attached example)
  2. This would effectively create a new with a suffix scaled from the original image, this image is stored as *.webp
  3. The full image is not stored on the site, with no fallback available.

This is the same as well if we upload a .webp image directly where no jpeg fallback would exist.

My suggested approach here would be similar to the sizes one which is to add a new sources property to the main image metadata (besides the sources). This key would be an array representing each a mime type:

if the original image is a large JPEG

  • Create a webp version from this JPEG version (so we can replace the full image with a webp) image.
  • Add the JPEG as part of the sources as well.

If the original image is a webp

  • Create a JPEG version and store it as part of the sources array.
  • Add the current webp as part of the sources array.

This would look like:

sources => array(
 'image/jpeg' => array( ... ),
 'image/web' => array( ... ),
)

A function to return the order of the sources (with filters) would exists as well so we can output the picture element in the right order.

function picture_mime_order() {
   return array( 
      'sources'' => 'image/web'
      'fallback'  => 'image/jpeg'
  );
}

Tihs would allow us to construct a picture element only using the meta from the image.

cc @adamsilverstein let me know if you think a new issue should be open to handle this scenario instead.

An image that can be used to replicate the scenario described above.

philip-myrtorp-hDBdvjk1b0s-unsplash

@adamsilverstein
Copy link
Member

While running some tests I face a new test case that would be worth considering how we are going to handle:

Good point about what happens when you upload a very large image and especially to consider large WebP uploads as well.

let me know if you think a new issue should be open to handle this scenario instead.

Yes, can you please open a new issue for this? I will do some testing with the image you provided (and the twentytwentytwo theme) in the meantime. Thanks!

@adamsilverstein
Copy link
Member

if the original image is a large JPEG

Create a webp version from this JPEG version (so we can replace the full image with a webp) image.
Add the JPEG as part of the sources as well.

I believe this is what happens currently when the webp-upload module is enabled.

@mitogh
Copy link
Member Author

mitogh commented Jan 21, 2022

Yes, can you please open a new issue for this? I will do some testing with the image you provided (and the twentytwentytwo theme) in the meantime. Thanks!

Ticket created:

I believe this is what happens currently when the webp-upload module is enabled.

Partially only *.webp version is generated. The suggested approach adds always a *.jpeg if does not exist already even for the largest size available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) [Type] Bug An existing feature is broken
Projects
No open projects
Development

No branches or pull requests

3 participants