wp-media / imagify-plugin

Speed up your website with lighter images without losing quality.
https://imagify.io
71 stars 25 forks source link

Closes 2554: Prevent having a webp bigger than the original #751

Closed CrochetFeve0251 closed 10 months ago

CrochetFeve0251 commented 11 months ago

Description

Fixes #2554

Display the message when we display a original format instead of the webp one.

Type of change

Please delete options that are not relevant.

Is the solution different from the one proposed during the grooming?

No.

Checklists

Generic development checklist

Test summary

If not, detail what you could not test.

Please describe any additional tests you performed.

Tabrisrp commented 11 months ago

We also have this PR related https://github.com/wp-media/imagify-plugin/pull/706

Mai-Saad commented 10 months ago

@CrochetFeve0251 Thanks for the update. Please find exploratory test notes below 1- The served image at the FE has size > the optimized size (for this image WebP isnot generated, however it's served, shouldn't we serve png in this case? even if the option to display WebP image on the website is enabled @wp-media/productimagify ) test page https://imagify.rocketlabsqa.ovh/test-webp/ Screenshot from 2023-11-08 09-23-55 Screenshot from 2023-11-08 09-22-06 2- WebP generated is always no although it was generated (@wp-media/productimagify what shall be the text for convert when the image is converted to WebP? do we really need this field? /cc @MathieuLamiot ) Screenshot from 2023-11-08 09-38-48 on trunk Screenshot from 2023-11-08 09-51-55

MathieuLamiot commented 10 months ago

@Mai-Saad, thanks for the detailed report. About the 2nd point:

I do agree it seems a bit redundant with the message above saying WebP is not generated because it is bigger. It could be a small simplification, but let's see what @wp-media/productimagify thinks about this. We might not have all the cases in minds. In any case, that would be outside the scope of the issue..

Mai-Saad commented 10 months ago

@CrochetFeve0251 Thanks for the update. @MathieuLamiot Thanks for the feedback. Currently:

Screenshot from 2023-11-09 18-06-32

Screenshot from 2023-11-09 18-05-46

Mai-Saad commented 10 months ago

@CrochetFeve0251 Thanks for the update. Working as the following now. If agreed by @wp-media/productimagify then we can merge. /cc @MathieuLamiot

  1. Original image is non webp

2- Original image is webP

3- Already on trunk, origional/ optimized size is not matching that in the upload folder https://wp-media.slack.com/archives/CU0F6EGQ1/p1699598198861799

4- Already on trunk, while display image in webp isnot checked, the used size at FE is that of webp but without adding webp extension https://wp-media.slack.com/archives/CU0F6EGQ1/p1699618969452739

DahmaniAdame commented 10 months ago

Looks good. The only change is on the wording to make it clearer about why the origin image will be served instead:

Change:

Webp is less performant than original

To:

WebP file is larger than the original image

Mai-Saad commented 10 months ago

Screenshot from 2023-11-13 15-58-06