Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
the crop_file_url_alter() is looking for a crop_type key in the image style effect configuration, but for a module provided crop type that extends ResizeImageEffect this is not a default key in the effect's configuration. Focal Point for example provides a crop that does not store the crop type in the image style configuration. Using the provider as a fallback we can check if there's a crop type with the provider name to set the value for passing to Crop::findCrop().
The attached patch does this.
Comment | File | Size | Author |
---|---|---|---|
#3 | crop_file_url_alter-2899173-3.patch | 3.6 KB | woprrr |
| |||
crop-file-url-alter-provider-crop-type.patch | 627 bytes | blake.thompson | |
|
Comments
Comment #2
blake.thompson CreditAttribution: blake.thompson at Forum One commentedComment #3
woprrr CreditAttribution: woprrr as a volunteer and at NeoLynk commentedHii @blake.thompson
Thank for this feedback. We can't consider that issue "Bug report" but more "Support Request" because in this case we need to add fallback for provider not using usual way (using manual crop effect) but using crop entity without his provided effect. This case seem's to be a little
less efficient than without fallback because we try to predict if providers match with crop_type and that tests fire a lot when we try to display an image using an ImageStyle.
I just process to a little extract method to add getCropFromImageStyle() at crop basis because that is really usefull for provider to evaluate if a crop match with a crop_type directly with combo uri / image_style and that permit to reduce the general complexity of hook in crop.module.
Please can you tell me if that works with your usecases/providers @blake.thompson
Comment #4
blake.thompson CreditAttribution: blake.thompson at Slalom commentedSo sorry, I finally got around to testing your patch. It looks like it does solve the issue if a crop type is not setting that value in its configuration. It looks like this issue was tackled within the Focal Point project as well https://www.drupal.org/node/2842260. Perhaps it just needs to be up to the module using Crop API to implement it correctly? At the same time it shouldn't hurt to have a fallback.
Comment #5
blake.thompson CreditAttribution: blake.thompson at Slalom commentedComment #8
woprrr CreditAttribution: woprrr as a volunteer and at NeoLynk commentedMerged Thanks all :) good job.