wp-media / imagify-plugin

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

Adjust priority for template_redirect #893

Closed wordpressfan closed 1 month ago

wordpressfan commented 3 months ago

Description

Fixes #891

Documentation

User documentation

As mentioned in the issue itself, here we fixed the priority of template_redirect hook's callback to be 10 instead of -1000 to fix the buffer closing conflict with WP Rocket.

Technical documentation

Just changed the priority.

Type of change

Delete options that are not relevant.

New dependencies

List any new dependencies that are required for this change.

Risks

As I mentioned here: https://github.com/wp-media/imagify-plugin/issues/891#issuecomment-2283827408

We added this -1000 priority in the mentioned PR to support rocket CDN so I tested this and it works so we may need to check another CDN plugin to see how it works with this branch.

Checklists

Feature validation

Documentation

Code style

Observability

Risks

Mai-Saad commented 3 months ago

@wordpressfan thanks for the PR. Here are the test results using imagify and WPR so far https://docs.google.com/spreadsheets/d/1YpkRpc_PkcszOezpkIooWC6hIWnkIWnXITvoHNjEjIo/edit?gid=0#gid=0 1- Are we going to handle the case in C6 here? 2- Is the o/p on C2 and C7 expected? 3- Do we need other GH about lcp loaded twice in NW and CDN not writing lcp totally? @piotrbak

Note: for row 2 , the behavior is different if we used LL plugin instead of enable the option in WPR , with LL plugin the lcp won't be excluded from LL at all. any idea why LL plugin behaves differently than LL in WPR ? (test page https://new.rocketlabsqa.ovh/gallery/)

Mai-Saad commented 3 months ago

@wordpressfan As per discussion with @piotrbak we need to handle the C6 related scenario 1- Enable display picture 2- Enable CDN, LL image 3- Visit page have no lcp yet => lcp is added to DB 4- Clear cache and revisit the page => lcp /atf shall be excluded from LL (currently it is not LL)

remyperona commented 1 month ago

Rocket lazyload plugin uses a priority of 2 on template_redirect, same as WP Rocket