wp-media / imagify-plugin

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

Compatibility issue after WordPress 6.4 with the new lightbox functionality #758

Open ionikol opened 10 months ago

ionikol commented 10 months ago

Describe the bug When the "Display images in WebP format on the site” option is turned on as tags and the new lightbox functionality on WordPress 6.4 is closed the site’s scroll gets stuck/broken.

To Reproduce Steps to reproduce the behavior:

  1. Update WordPress 6.4 to have the new lightbox feature
  2. Enable lightbox on an image
  3. Have the "Display images in WebP format on the site" turned on as tags
  4. Click on image on frontend to open lighbox
  5. Click X in top right of image to close lightbox
  6. Can't scroll up/down even though lightbox closed

Additional context Tickets from HS and WordPress.org https://secure.helpscout.net/conversation/2416490954/453777/ https://wordpress.org/support/topic/imagify-conflicts-with-wordpress-6-4s-new-lightbox-functionality/

Acceptance Criteria (for WP Media team use only) Clear instructions for developers, to be added before the grooming

piotrbak commented 10 months ago

It can be seen here: https://rocketlabsqa.ovh/testing-image-lightbox/

CrochetFeve0251 commented 10 months ago

Root cause

The root cause is that we are adding the attributes on both the img tag and picture one.

Scope a solution

A simple solution would be to filter out WordPress attributes for the picture tag in classes/Webp/Picture/Display.php:

        $p_attributes = array_filter($attributes, function ($attribute) {
            return ! str_contains($attribute, 'data-wp');
        }, ARRAY_FILTER_USE_KEY);

        $output = '<picture' . $this->build_attributes( $p_attributes ) . ">\n";

Estimate effort

Effort XS

jeawhanlee commented 10 months ago

Looks good to me.

CrochetFeve0251 commented 10 months ago

@MathieuLamiot @engahmeds3ed is checking how the interactivity package is working inside Guttenberg as adding a callback to the lightbox doesn't seems to be attached to the context

wordpressfan commented 9 months ago

There is a progress there in Gutenberg for this issue, they opened a new PR to solve that: https://github.com/WordPress/gutenberg/pull/57089

I'll test it with this PR but I believe it'd work, hope they release it soon.

MathieuLamiot commented 8 months ago

@piotrbak Gutenberg 17.4 is not released yet (RC01 ongoing). Should this issue be delayed to Imagify 2.1.5 then?

piotrbak commented 8 months ago

@MathieuLamiot @CrochetFeve0251 Just to confirm, when Gutenberg releases the enhancement, will it fix the issue automatically or it'll allow us to fix it on our end?

MathieuLamiot commented 8 months ago

@wordpressfan @CrochetFeve0251 ?

wordpressfan commented 8 months ago

We still need this PR to be merged to add the attributes to pictures. The Gutenberg PR that will be shipped with v17.4 will fix the console error that Vasilis mentioned here: https://github.com/wp-media/imagify-plugin/pull/762#issuecomment-1825255946

rfischmann commented 3 months ago

So this is still an issue today?

soloman981 commented 3 months ago

So this is still an issue today?

Yes it is :)

wordpressfan commented 1 month ago

@rfischmann @soloman981 I can't replicate it from my side, can you please share the WP and imagify versions being used from your side to try replicating it? Thanks.