imagify-plugin icon indicating copy to clipboard operation
imagify-plugin copied to clipboard

Compatibility issue after WordPress 6.4 with the new lightbox functionality

Open ionikol opened this issue 1 year ago • 12 comments

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

ionikol avatar Nov 16 '23 10:11 ionikol

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

piotrbak avatar Nov 17 '23 14:11 piotrbak

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

CrochetFeve0251 avatar Nov 21 '23 09:11 CrochetFeve0251

Looks good to me.

jeawhanlee avatar Nov 21 '23 14:11 jeawhanlee

@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

CrochetFeve0251 avatar Nov 30 '23 14:11 CrochetFeve0251

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.

wordpressfan avatar Dec 15 '23 12:12 wordpressfan

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

MathieuLamiot avatar Jan 02 '24 16:01 MathieuLamiot

@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?

piotrbak avatar Jan 02 '24 16:01 piotrbak

@wordpressfan @CrochetFeve0251 ?

MathieuLamiot avatar Jan 03 '24 17:01 MathieuLamiot

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

wordpressfan avatar Jan 03 '24 18:01 wordpressfan

So this is still an issue today?

rfischmann avatar Jun 26 '24 12:06 rfischmann

So this is still an issue today?

Yes it is :)

soloman981 avatar Jun 27 '24 13:06 soloman981

@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.

wordpressfan avatar Aug 12 '24 11:08 wordpressfan