Contrib.social

Crop API is not appending a hash when the same image is cropped with multiple image crop styles

Open on Drupal.org →
Created on 20 January 2022, over 3 years ago
Updated 9 May 2024, 12 months ago

Problem/Motivation

Today we have a website using Crop API and Image Widget Crop → . We created an image style with some "crop types" effects, and set it inside our image field.

The result is something like:

The problem is, if I crop something inside 4:3 Crop, save and publish the node, the hash will not be appended on the URL, and this is a important feature for the website because it's using a CDN.

FYI: Check crop_file_url_alter for more details about the hash and it's implementation.

Steps to reproduce

  1. Install a new instance of drupal 9.
  2. Require and install crop api module:

    3. composer require 'drupal/crop:^2.1'
    drush en crop

  3. Require and install image_widget_crop module:

    4. composer require 'drupal/image_widget_crop:^2.3'
    drush en image_widget_crop

  4. Create at least two new crop types

    5. Go to /admin/config/media/crop and click on Add crop type

  5. Add a new image style with the crops effects

    6. Go to /admin/config/media/image-styles/add and a "test image style".
    On the next screen, select Manual Crop and add the crops created on the step 4.

  6. If you're using a new drupal instance, you probably have the Article content type.

    7. So go to the content type form display /admin/structure/types/manage/article/form-display and change the image field type to use ImageWidget crop.

  7. Now you need to change the manage display of the content type.

    8. Go to /admin/structure/types/manage/article/display and change the Image field to use the new image style created on step 5.

  8. Create a new content with an image, you'll have to choose between two crop types, if you choose the first one it will not work as expected, just the last crop type will work as expect and append the hash to the URL.

Proposed resolution

Currently the code have the function getEffectsFromImageStyleId which is responsable for retrieving all the effects from the image style, the main problem is, the code is expecting just one plugin of crop_crop and it's always overriding the array. If you have more than one crop type on the image style, just the last crop will be added to the $effects variable.

The proposed solution here is to create a multi dimension array containing all the crop_crop effects, this way getCropFromImageStyleId is able to find the correct crop and the hash will be attached.

Remaining tasks

  2. Review

API changes

The \Drupal\crop\Entity\Crop::$effectsByImageStyle will have an array of effects.

🐛 Bug report
Status

Needs work

Version

2.0

Component

Code

Created by

🇧🇷Brazil murilohp

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • First commit to issue fork.
  • Status changed to RTBC 12 months ago
  • Comment 12 months ago →
    🇺🇦Ukraine sickness29

    Hi @murilohp
    image widget crop does not allow to add multiple crop types per image style and it does not make sense.
    Steps to reproduce is not accurate as you need to hack module code to make 5th point work

    Hi @phenaproxima
    please check my comment and move issue status to fixed if you agree. Thanks

  • Comment 11 months ago →
    🇫🇷France DrDam

    @sickness29 :
    In fact, it's a limitation of the Crop Entity it self.

    I actually have a real use case for this type (particularly when you want to render several versions of the image via responsive).

    - You can define 2 distinct image styles: "16-9-large" and "16-9-thumbnail" (both using the same "16-9" CROP-TYPE). The difference is in the dimensions displayed (one 1k px wide, the other x00 px wide).
    - You can render the image in the 2 styles separately (using the image formatter or using responsives images).
    - For ALL the other styles, you can choose an area of interest different to that of 16-9 CROP-TYPE (That's main adventage in frontof focal-point).
    - But if your 2 images use the same type of CROP, you can no longer have 2 different aera of interest (one for each style).

    The "classic method" for handling this case consists of creating a CROP-TYPE for each variant of the "same CROP-TYPE" (on for the "16-9-large" and another for "16-9-thumbnail"), and finally have a CROP-TYPE for each image style that uses a CROP.

    What is the point of distinguishing the CROP-TYPE from the image style if the global solution are to have a 1-1 relation ?

  • Comment 5 months ago →
    🇧🇷Brazil peduardo

    At the line 184 I just put an isset to validate if the key type is there

    if (isset($effect['type']) && $crop = self::findCrop($uri, $effect['type'])) {
    return $crop;
    }

  • Comment 5 months ago →
    🇷🇺Russia zniki.ru

    Put back to NW, still need tests.

  • Status changed to Needs work about 2 months ago
  • Comment about 2 months ago →
    🇫🇷France DrDam

    It seems it was corrected in ImageWidgetCrop 3.x

    On a fresh drupal install
    - I have create 2 crop type
    - On a unique image style, I add 2 effect "manual crop" (one for each crop)
    - On the article mange display interface, I choose the image style which use the both crop
    - On the article manage form display, I choose "ImageWidget Crop" widget. It only propose the "last crop type" for selection.

    see 3259571-IWC.png & 3259571-imageStyles.png

    I propose to close this issue

  • Comment about 2 months ago →
    🇫🇷France DrDam
  • Comment about 2 months ago →
    🇫🇷France DrDam
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024