Crop API is not appending a hash when the image styles are converted to WEBP

This patch with working for me. Thanks @kaszarobert!

Does this impact those using the contrib WebP → module and not using core's "Convert" image style effect?

Implementation looks OK now. Uses camelCase for the variable name, doesn't really fit in with the rest in the same function (coding standards say you can use camelCase, but the whole file should be consistent). But that's minor.

A test for this would be good. based on our tests, this is also required for the core convert option, so it should be possible adjust/duplicate the existing tests when they also have a convert operation.

Confirming RTBC, in my case crop_file_url_alter was adding a h query parameter and then FileUrlGeneratorInterface::generateAbsoluteString was not adding the itok param because there was something already in there.

Hello there.

I'm using these two lines in settings.php:

$config['image.settings']['suppress_itok_output'] = TRUE;
$config['image.settings']['allow_insecure_derivatives'] = TRUE;

Does it matter for this patch?

Now, despite your patch, converting to WEBP still makes crop disappear (using a cropped image in a view).

D 10.2.2, Crop API 8.x-2.3, ImageWidgetCrop 8.x-2.4, Image Effects 8.x-3.6.

Thanks!

Szy.

Patch is working great, thanks!

I can confirm, patch from #4 works perfect, thanks!

I have this issue with conversion to PNG too.
Therefore I propose a more generic solution to pass the conversion type directly in to the function.
I haven't tested this patch with Webp-Files.

Hi,

I think I've found a solution that I think is ‘simpler’ (less intrusive and based on what the core does in generating thumbnails).

The main idea is to retrieve the code from the ImageStyleDownloadController::deliver :

    // Don't try to generate file if source is missing.
    if (!$this->sourceImageExists($image_uri, $token_is_valid)) {
      // If the image style converted the extension, it has been added to the
      // original file, resulting in filenames like image.png.jpeg. So to find
      // the actual source image, we remove the extension and check if that
      // image exists.
      $path_info = pathinfo(StreamWrapperManager::getTarget($image_uri));
      $converted_image_uri = sprintf('%s://%s%s%s', $this->streamWrapperManager->getScheme($derivative_uri), $path_info['dirname'], DIRECTORY_SEPARATOR, $path_info['filename']);
      if (!$this->sourceImageExists($converted_image_uri, $token_is_valid)) {

and inject this controle in the crop::getCropFromImageStyleId method

In Drupal 10.3+ , we have a new static method to do that

$converted_original_uri = ImageStyleDownloadController::getUriWithoutConvertedExtension($original_uri);

I'm open a merge request to add this

I don't exactly know how it works, but the solution from #15 helped me out.

My problem was Cloudflare which was displaying the old image even after Focal point was saved (using Focal point essentially crops the image). And by adding the fix from #15 the image uri receives the missing ?h parameter.
I'm also converting the image to webp format which seems to be one of the reasons for this problem. Without webp conversion the ?h parameter appears, but with the patch I'm able to get the ?h parameter there even with webp conversion enabled.
I'm using Drupal 10.3.9 and Crop API 8.x-2.4.

Thanks @hartsak, I pass the ticket to RTBC

nikolay shapovalov → made their first commit to this issue’s fork.

I create test to demonstrate the issue, created Draft MR #21 to run pipelines.
Also applied changes from MR #21 to MR #20 to test changes.

My suggestion to create followup to refactor CropFunctionalTest::doTestFileUriAlter().

$this->assertTrue(strpos($image_style_url, $shortened_hash) !== FALSE, 'The image style URL contains a shortened hash.');
// Suggestion.
$this->assertStringContainsString($shortened_hash, $image_style_url, 'The image style URL contains a shortened hash.');

Hide patch files.