arnowelzel / lightbox-photoswipe

Integration of PhotoSwipe to WordPress
https://arnowelzel.de/wp/en/projects/wordpress/lightbox-with-photoswipe
GNU General Public License v2.0
27 stars 7 forks source link

Incompatibility with EWWW Image Optimizer (ExactDN) #82

Closed JanikWeb closed 2 years ago

JanikWeb commented 2 years ago

Hi there,

we use ExactDN to deliver optimized images from a CDN.

https://github.com/nosilver4u/ewww-image-optimizer

This plugin filters the_content and removes all data-attributes added by lightbox-photoswipe.

Is there any workaround to delay the lightbox filter function so it runs after the ExactDN filter?

arnowelzel commented 2 years ago

I'd consider this a bug in ExactDN. Did you already contact the maintainer of EWWW Image Optimizer? Because I use this plugin as well and it works fine here, but without "ExactDN" of course.

A possible workaround might be to change the priority in https://github.com/arnowelzel/lightbox-photoswipe/blob/main/lightbox-photoswipe.php#L125. Try a higher number like 5000 or 10000 instead of 2050. A sledge hammer method would be to use PHP_MAX_INT to be sure that this filter will run as late as possible.

However - I did not use such a high priority on purpose, as this WILL cause problems with caching plugins! So I can not add a higher number for the regular version. Mayb I'll add a backend configuration option to change the filter priority if needed along with a warning that his can cause other problems.

JanikWeb commented 2 years ago

I'd consider this a bug in ExactDN.

That's what I thought.

A possible workaround might be to change the priority in https://github.com/arnowelzel/lightbox-photoswipe/blob/main/lightbox-photoswipe.php#L125. Try a higher number like 5000 or 10000 instead of 2050. A sledge hammer method would be to use PHP_MAX_INT to be sure that this filter will run as late as possible.

I tried all three options, sadly without any luck. The ExactDN filters seem to be pretty aggressive.

Thanks for the help, Arno. I'll try my luck directly on their github page and open a new issue.

nosilver4u commented 2 years ago

Hi @arnowelzel, I'm pretty confident that our Easy IO (ExactDN) parser isn't removing attributes, so I was wondering if it were possible that they just aren't getting added in the first place?

I installed the plugin myself, and noticed (as pointed out by @JanikWeb also) that it has a CDN setting which seems specifically for making sure your plugin can resolve CDN URLs back to local file paths. However, when the plugin is active, it seems to work perfectly with our Easy IO CDN, and the lightbox pops up just as expected (on linked images).

Then I noticed there is a setting to "ignore links to images on other sites", so I checked that, and the lightbox no longer worked, and the data-* attributes were gone. I added my CDN domain: https://earqb5e5sei.exactdn.com (@JanikWeb said he did this also), and the lightbox still doesn't work. Perhaps the lightbox plugin needs to strip query strings (similar to Jetpack/Photon) so that it can properly find the local images?

JanikWeb commented 2 years ago

Thanks @nosilver4u for participating here. I need to mention that everything works fine when the images are beeing returned via get_thumbnail() etc from inside a custom page template. See here: https://www.metal1.info/konzertfotos/gruzja-josefstadt-2021/

Only images embedded via the wp editor (linking to media file) aren’t working. See here: https://www.metal1.info/konzerte/kreator-w-eluveitie/

nosilver4u commented 2 years ago

@JanikWeb That's not too surprising, as EWWW IO hooks in at several low level WP functions for URL rewriting, so the Lightbox plugin might encounter some URLs that are still local, and some that are rewritten for the CDN.

arnowelzel commented 2 years ago

@nosilver4u Thanks for taking the time to find possible issues here.

In general - the image needs to be in the form http[s]://WEB-SITE-DOMAIN/wp-content/uploads/.../IMAGE-NAME or http[s]://CDN-URL/wp-content/uploads/.../IMAGE-NAME. If the image name is somewhere inside the URL and not at the end (query strings and fragments don't matter) it will not work.

About query strings: these are stripped already: https://github.com/arnowelzel/lightbox-photoswipe/blob/main/lightbox-photoswipe.php#L520 and are no problem at all. Furthermore the CDN URLs are replaced before the image URL is considered to be "local" or not, see https://github.com/arnowelzel/lightbox-photoswipe/blob/main/lightbox-photoswipe.php#L539-L552

However - if multiple URLs for the CDN are used, you need to add all possible variations. If the CDN uses a more complex variation I might add an option to use regular expressions for the CDN URL similar to the already included Jetpack.

If EWWW would provide some method to translate the real URL from the HTML code back to the original image URL I'd use this as well if needed.

nosilver4u commented 2 years ago

Hmm, well so much for my bright idea :) At any rate, the lightbox is working for me when the CDN is active, but only breaks when enabling "ignore links to images on other sites", so that would seem to be a glitch somewhere along the way.

I'm not sure I fully understand what's happening at https://github.com/arnowelzel/lightbox-photoswipe/blob/5d16e931b8e09cbb2d94d459581471dead3a703f/lightbox-photoswipe.php#L544

For this image as an example: https://earqb5e5sei.exactdn.com/wordpress/wp-content/uploads/2020/11/IMG_0944.jpg?strip=all&lossy=1&ssl=1 and a CDN URL prefix of https://earqb5e5sei.exactdn.com That would give a path something like this, right? http:///wordpress/wp-content/uploads/2020/11/IMG_0944.jpg

Somehow that doesn't seem right, but you'd know your code better. Is that expected?

nosilver4u commented 2 years ago

I forgot to mention, since our CDN uses a standard "pull-mode" URL format, (rather than the "prefixed" method used by folks like ShortPixel, Jetpack, etc.) you can just swap out the domain to get the local URL. No special functions needed :)

arnowelzel commented 2 years ago

I think this "pull-mode" format is the important hint here!

So far I only expected CDNs to add their own URL as a prefix like Jetpack etc.. So what is needed here is to add the domain of the website if the image URL contains the CDN URL but not the domain of the website itself - because in this case the image is not considered to be local and will be ignored if you enable "ignore links to images on other sites". Furthermore the code will also not be able to get the image meta data since this is only done for "local" images.

It would help a lot if I can have some kind of test environment for this. @JanikWeb or @nosilver4u - can you provide a test site to try out a beta version with the changes in Lightbox with PhotoSwipe when they are ready to test?

JanikWeb commented 2 years ago

Sure, I can give you access to our staging site. I’ll text you the details later.

arnowelzel commented 2 years ago

Release 3.2.4 should fix this.