qtranslate / qtranslate-xt

qTranslate-XT (eXTended) - reviving qTranslate-X multilingual plugin for WordPress. A new community-driven plugin soon. Built-in modules for WooCommerce, ACF, slugs and others.
GNU General Public License v2.0
553 stars 104 forks source link

[qTranslate-XT 3.15.0] conflict with plugin [Lightbox with PhotoSwipe] #1333

Closed bagaweb closed 1 year ago

bagaweb commented 1 year ago

After updating qTranslate-XT to release 3.15.0 this error appears:

`Fatal error: Uncaught TypeError: Argument 2 passed to QTX_Module_Slugs::home_url() must be of the type string, null given, called in ...\wp-includes\class-wp-hook.php on line 308 and defined in ...\wp-content\plugins\qtranslate-xt\src\modules\slugs\slugs.php:531 Stack trace:

0 ...\wp-includes\class-wp-hook.php(308): QTX_Module_Slugs->home_url('http://localhos...', NULL, 'http', NULL)

1 ...\wp-includes\plugin.php(205): WP_Hook->apply_filters('http://localhos...', Array)

2 ...\wp-includes\link-template.php(3445): apply_filters('home_url', 'http://localhos...', NULL, 'http', NULL)

3 ...\wp-content\plugins\lightbox-photoswipe\src\LightboxPhotoSwipe\LightboxPhotoSwipe.php(274): get_home_url(NULL, NULL, 'http')

4 [internal function]: LightboxPhotoSwipe\LightboxPhotoSwipe->callbackProperties(Array)

5 ...\wp-content\plugins\lightbox-photoswipe\src\LightboxPhotoSwipe\LightboxPhotoSwipe.php(606): preg_ in ...\wp-content\plugins\qtranslate-xt\src\modules\slugs\slugs.php on line 531`

bagaweb commented 1 year ago

The error disappears if I replace this file:

qtranslate-xt\src\modules\slugs\slugs.php

with the one in 3.14.2 release.

herrvigg commented 1 year ago

I expected this kind of issues (see PHP typing in changelog). But this should be easy to fix in 3.15.0. Try changing string $path to ?string $path (to allow null) here in slugs: https://github.com/qtranslate/qtranslate-xt/blob/1bce4a3e106f7998d723d99f548f315e9bb88c9a/src/modules/slugs/slugs.php#L531

The source problem though is in LightBox PhotoSwipe: https://github.com/arnowelzel/lightbox-photoswipe/blob/3fde62402ef03760d385003191e26346fc0feaf2/src/LightboxPhotoSwipe/LightboxPhotoSwipe.php#L274

This is a wrong usage from their side. They call the WordPress function get_home_url(null, null, 'http') but this should not be allowed by the signature: https://developer.wordpress.org/reference/functions/get_home_url/.

bagaweb commented 1 year ago

Thanks @herrvigg !

I followed your suggestion and changed string $path to ?string $path and the error went away.

Do you think I should open an issue on LightBox PhotoSwipe GitHub?

herrvigg commented 1 year ago

Good. I opened a ticket already - https://github.com/arnowelzel/lightbox-photoswipe/issues/98.

bagaweb commented 1 year ago

Super-efficient!!

herrvigg commented 1 year ago

I also expected these PHP type checks to find some bugs like this. If WordPress did the same we would be in a much better shape but they would have a huge amount of technical debt to deal with.

herrvigg commented 1 year ago

I revert the fix in qTranslate-XT since it's already been fixed in LightBox PhotoSwipe 5.0.34. See commit https://github.com/arnowelzel/lightbox-photoswipe/commit/9209ff1c3e2379b99fb63aeeebf2ae9e13e0dea4.

bagaweb commented 1 year ago

Excellent, I confirm everything works flawlessy!

Thank you :)