junaidbhura / fly-dynamic-image-resizer

Fly Dynamic Image Resizer plugin for WordPress
https://wordpress.org/plugins/fly-dynamic-image-resizer/
MIT License
162 stars 26 forks source link

Potential conflict with Performance Lab plugin #57

Open cjhaas opened 1 year ago

cjhaas commented 1 year ago

I'm still fleshing out the details of this, but wanted to list an issue just in case anyone else ran into this.

We have a site with both Fly Dynamic Image Resizer and Performance Lab. For some images, the call WP_Image_Editor::save() (specifically WP_Image_Editor_GD::save() in our case) are having JPEGs turned into WebP files automatically by the PL plugin, however FDIR isn't accounting for this.

In the get_attachment_image_src there's a called to save() with just the filename, no MIME. https://github.com/junaidbhura/fly-dynamic-image-resizer/blob/57fc985b8e378c74f4a8aaebedd7a6e5d83ceea6/inc/class-core.php#L297

The save() method then calls _save().

When not passing a MIME, the default used to be to be to figure it out based on file extension via that get_output_format method. This is still the case, however as of 5.8 the results of that are then passed to the image_editor_output_format hook which the PL plugin uses:

https://github.com/WordPress/performance/blob/86a7776df8927c01f886647bbdd0e166731fa9c9/modules/images/webp-uploads/hooks.php#L272-L292

The problem appears to be that when FDIR calls save() on an image, it assumes that the image path and/or type it provides to WordPress will be the same, however save() actually returns an array with the results of the call which might include different information. The output format potential change has been going on for a while now, but I think it really only accounted for files with missing or wrong extensions, the changing of the MIME type is new-ish.

Like I said at the beginning, we're still ironing things out but for now we've just disabled the Performance Lab plugin, that immediately corrected the error. (We could just disable that feature probably, too.)

I don't really know if this a technically a bug with FDIR, either, but we did lose almost two hours debugging this, so we thought we'd raise a flag at least.

Once I get time, I'll see if I can work up a proof-of-concept that shows the issue, along with a supporting image. (The problem wasn't all images, either, which made it harder to debug.)

cjhaas commented 1 year ago

Just an update that right now we are adding this to our code to get around this:

add_filter(
    'image_editor_output_format',
    static function ($mappings, $filename, $mime_type) {
        return [];
    },
    10,
    3
);

This is probably a shotgun approach but it does resolve the issue