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
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | 3213379-12.patch | 1.08 KB | idebr |
Comments
Comment #2
kaszarobertI 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.
Comment #3
kaszarobertA 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.
Comment #4
dennis cohn commentedWe've applied the patch and it works perfectly!
Comment #5
dennis cohn commentedComment #6
tvalimaa commentedPatch #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.
Comment #7
maxpahHello,
Thanks for the patch, just a point, I think this line is not good :
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.
Comment #8
kaszarobertRight 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.
Comment #9
michaellander commentedI'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?
Comment #10
kaszarobertThe regenerating code should run after the
\Drupal\image\Entity\ImageStyle::flush($path)is done.1. If you pass the image
$pathparameter, 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
$pathparameter, then it flushes all the files in the current ImageStyle's folder, and calls theimage_style_flushhook. 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.Comment #11
idebr commented#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.
Comment #12
idebr commentedThe 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.
Comment #13
anneke_vde commentedI tested the patch from #12, the webp image is now changed if I choose a new crop.
Comment #14
mjpa commentedThe 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.
Comment #15
daniel korte@mjpa #12 requires the patch from #2833129: hook_image_style_flush doesn't get the $path passed to Drupal\image\Entity\ImageStyle::flush() method
Comment #16
useernamee commentedI've tested patch #12 together with the image style hook and it works. I can confirm RTBC status.
Comment #17
steveoriolI have the impression that the patch does not work on the latest version of Drupal D9.4.5
Comment #18
weseze commentedPatch from #12 + https://www.drupal.org/project/drupal/issues/2833129#comment-14303948 + cache clear.
Works perfect for me on D9.4.7.
Comment #19
vistree commentedPatch from #12 + https://www.drupal.org/project/drupal/issues/2833129#comment-14303948 works perfectly for me on Drupal 9.4.9
Comment #20
martijn de witCore patch is committed #2833129: hook_image_style_flush doesn't get the $path passed to Drupal\image\Entity\ImageStyle::flush() method
Comment #21
dieterholvoet commentedClosed #3362476: Webp image derivatives are not deleted when normal image derivatives are being flushed as a duplicate.
Comment #22
akz commentedI've tested patch #12 in Drupal 10.2.7 and it works. I confirm RTBC status.
Comment #24
michaellander commented