wp-media / imagify-plugin

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

Update callback priority on template_redirect for picture display #891

Open Tabrisrp opened 1 month ago

Tabrisrp commented 1 month ago

Context Related to https://github.com/wp-media/wp-rocket/issues/6786

WP Rocket Lazyload exclusion is not working correctly when excluded pattern is part of a picture element created by the picture rewriting feature of Imagify.

Expected beavhior Lazyload exclusion should work as expected

Additional information So I believe I found what was happening with the next-gen issue. we have 2 callbacks on template_redirect:

So we should expect imagify to be finished first, and WP Rocket after. BUT, both callbacks have an ob_start() call, and when multiple are called, they become nested until the buffering stops:

When output buffering ends (on shutdown action of WP), lower level content is passed to upper, so the order becomes WP Rocket optimizations first, then Imagify second, and HTML is output. While callback priority is as expected, the output buffering works in the opposite order.

To verify, I tested by changing the priority of Imagify callback to 10, reverting both orders, and it works as expected, the picture element is applied first in the HTML, and then lazyload exclusion works as needed.

wordpressfan commented 1 month ago

I can confirm that the provided fix is working from my side, I was trying to find why we used to use this priority -1000 and I found that we did that here:

https://github.com/wp-media/imagify-plugin/pull/383 to fix this issue: https://github.com/wp-media/imagify-plugin/issues/368

So I believe we need to validate that issue and make sure that CDN is working properly with such change.

johan-las commented 1 month ago

New related case: https://secure.helpscout.net/conversation/2676079310/507022?folderId=8127840

wordpressfan commented 1 month ago

I tested when using RocketCDN here: https://new.rocketlabsqa.ovh/next-gen-image/ and all good I believe, so we are fine here.

MathieuLamiot commented 3 weeks ago

Blocked pending product decision on the feedbacks from QA.

MathieuLamiot commented 3 weeks ago

Feedbacks from QA on the PR need to be investigated then handled. this is not a priority compared to 3.17 os moving this back to Needs Grooming and outside of the sprint.

alfonso100 commented 1 day ago

related https://secure.helpscout.net/conversation/2701935175/511314/#thread-8146661296