Problem/Motivation

If I apply a crop for an image, it crops the jpg and webp images fine.
But if I change the crop again, only the image style's jpg files are regenerated, the webp remains the same.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kaszarobert created an issue. See original summary.

kaszarobert’s picture

Status: Active » Needs review
FileSize
1.29 KB

I attach a patch for the fix. After applying the patch, drush cr is needed.

The crop module (that the image_widget_crop is depending on) has a postSave function where it removes all the generated stylized image files for that image if I update a crop settings for that field. In this patch I add an entity update hook where if a crop entity is updated for an image, imageapi_optimize_webp removes all the generated webp images in every image style for that image. That way, next time when it's loaded in the browser, it'll be regenerated on the fly with using the new cropping settings.

That patch should also fix every filter's regeneration that is based on the crop module (such as focal point). Although I didn't test those, theoretically it should work.

kaszarobert’s picture

A new patch: I added another thing: regenerate webp when the crop is created. If an image field already contains an image file and the crop widget is set in the form display later, then the applied crop settings will create a new crop entity. And if there's only an update hook and no insert hook for that crop entity, then the generated image styles won't be regenerated until I actually update an existing crop.

Dennis Cohn’s picture

We've applied the patch and it works perfectly!

Dennis Cohn’s picture

Status: Needs review » Reviewed & tested by the community
tvalimaa’s picture

Patch #3 solved my issue with focal point. Before patch when images focal point was changed node was showing old image and if I open image url on a new tab it open right version of image. Also focal point was working correct when I open node edit.

MaxPah’s picture

Hello,

Thanks for the patch, just a point, I think this line is not good :

$path = $image_uri . '.webp';

The URI is something like "public://mypath/image.jpg" and with the above code will be "public://mypath/image.jpg.webp" which is wrong.

The patch attached works better for me.

kaszarobert’s picture

Right now, the module generates path in ImageAPIOptimizeWebPPipeline and WebP classes like this: mypic.jpg.webp.

Patch #7 won't work properly when the webp picture is already generated with the current naming. It is because it flushes image styles that have the path like that: mypic.webp and if the image was already generated in that style, then it has a path like that: mypic.jpg.webp.

First, issue #3203988: Don't add original file extension in webp versions of images needs to be fixed for patch #7 to work. Until that happens, patch #3 is advised to use.

michaellander’s picture

Status: Reviewed & tested by the community » Needs work

I'd like to see if there's a way to do this without implementing a hook specific to the crop module. This pattern isn't sustainable for other modules which may have similar functionality. Is it possible for us to hook into the regeneration of the source image without being module specific?

kaszarobert’s picture

The regenerating code should run after the \Drupal\image\Entity\ImageStyle::flush($path) is done.

1. If you pass the image $path parameter, then flushes that image's file in the current ImageStyle. This code is called in Drupal and contrib. The function doesn't create an event or calls a hook when that happens, so we cannot subscribe to anything then.
2. If you don't pass a value in the $path parameter, then it flushes all the files in the current ImageStyle's folder, and calls the image_style_flush hook. But that only happens when the image style configuration is changed, so rarely.

The only solution would be to alter the ImageStyle config entity class with a new one in this module. In this class we would only modify the flush() method by extending its code to flush the webp path.

idebr’s picture

Status: Needs work » Needs review
FileSize
749 bytes

#10 requires a patch for Drupal Core: #2833129: hook_image_style_flush doesn't get the $path passed to Drupal\image\Entity\ImageStyle::flush() method

Attached patch flushes the webp version of an image style when a specific image style derivative is flushed, for example (but not limited to) when changing a crop.

idebr’s picture

FileSize
1.08 KB

The patch in #11 results in a lot of additional hook invocations. Attached patch calls only the relevant part of ImageStyle->flush() to limit the performance impact.

anneke_vde’s picture

Status: Needs review » Reviewed & tested by the community

I tested the patch from #12, the webp image is now changed if I choose a new crop.

mjpa’s picture

The patch in #3 works for me. The patch in #12 does not work for me.

Test case for me is editing a media item, change the focal point on an image field and save. The hook in #12 doesn't get called.

Daniel Korte’s picture

useernamee’s picture

Issue tags: +image styles

I've tested patch #12 together with the image style hook and it works. I can confirm RTBC status.

steveoriol’s picture

I have the impression that the patch does not work on the latest version of Drupal D9.4.5

weseze’s picture

Patch from #12 + https://www.drupal.org/project/drupal/issues/2833129#comment-14303948 + cache clear.

Works perfect for me on D9.4.7.

vistree’s picture

Patch from #12 + https://www.drupal.org/project/drupal/issues/2833129#comment-14303948 works perfectly for me on Drupal 9.4.9

Martijn de Wit’s picture