Problem/Motivation
Currently, when an image is converted to another format by an image style, the new extension is appended to the file name (instead of the new extension replacing the old extension).
Details
The extension is added by Drupal\image\EntityImageStyle::addExtension(), which also does a pretty good job of explaining what is happening and why:
/**
* Adds an extension to a path.
*
* If this image style changes the extension of the derivative, this method
* adds the new extension to the given path. This way we avoid filename
* clashes while still allowing us to find the source image.
*
* @param string $path
* The path to add the extension to.
*
* @return string
* The given path if this image style doesn't change its extension, or the
* path with the added extension if it does.
*/
protected function addExtension($path) {
$original_extension = pathinfo($path, PATHINFO_EXTENSION);
$extension = $this->getDerivativeExtension($original_extension);
if ($original_extension !== $extension) {
$path .= '.' . $extension;
}
return $path;
}
Then, when an image derivative is requested (assuming it doesn't already exist and would then be served by the web server), the request is first processed by Drupal\image\PathProcessor\PathProcessorImageStyles::processInbound(). This path processor rewrites the request URI and appends a query parameter for the original image file.
If the $path parameter passed to ::processInbound() is '/sites/default/files/styles/blog_article_image_1792x784/public/blog/article-image/image.png.webp', then the method will return
'/sites/default/files/styles/blog_article_image_1792x784/public' with the request modified by adding a 'file' query parameter value of 'blog/article-image/image.png.webp'.
The request is later handled by Drupal\image\Controller\ImageStyleDownloadController::deliver(). The $scheme and $image_style parameters are parsed from the path. (In this example, $scheme is 'public' and $image_style is 'blog_article_image_1792x784'.) The 'file' query parameter is used to infer the original file.
As a result, implementing this feature request will require touching several pieces of code. The issue of namespace collisions will also need addressed. E.g. What happens if image.png and image.jpg are both uploaded? Both can't use the image.webp namespace.
As is detailed in #3412154: ImageStyleDownloadController::deliver() infers source image when generating derivatives with image format conversion, Drupal\image\Controller\ImageStyleDownloadController::deliver() can incorrectly infer the source image, and, as a result, render an incorrect image.
Because this behavior is intentional, I'm filing this issue as a feature request instead of a bug.
Steps to reproduce
- Install a vanilla Drupal site with the 'Standard' profile.
- Install Media & Media Library modules.
- Navigate to /admin/config/media/media-settings and enable the 'Standalone media URL' setting.
- Ensure the 'Default' view mode for the 'Image' media type is configured to render the 'Large' image style (/admin/structure/media/manage/image/display).
- Update the 'Large' image style to convert to 'WebP' (/admin/config/media/image-styles/manage/large).
- Upload a PNG image (e.g. image.png) to a media entity (/media/1).
- Load /media/1 and observe the image file name is image.png.webp and not image.webp.
Proposed resolution
TBD
Remaining tasks
TBD
User interface changes
TBD
API changes
TBD
Data model changes
TBD
Release notes snippet
TBD
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | image_style_replace_extension_not_append.patch | 10.05 KB | mukeshaddweb |
Issue fork drupal-3413632
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
candelas commentedGood to learn about this bug. I was going to convert all images to webp.
I hope you to have time to make a patch.
Thanks
Comment #3
cilefen commentedThis looks like a bug report rather than a feature request.
Comment #4
bramvandenbulcke commentedI have been using the conversion to webp functionality since launch (in core). I have seen this behaviour from the beginning. But I thought this was intentional. Apparently not!
Comment #7
w01f commentedremoved as probably not helpful
Comment #8
dries arnoldsI found that some web scrapers that fetch article previews choke on the double file extension that this problem describes. I tried to move a websites' image styles to incorporate conversion to webp, but Reddit and other sites wouldn't fetch the main article image anymore.
Comment #10
guardiola86 commentedIt's not working for me. Although the files no longer have two extensions, the image styles are not being generated.
Comment #11
guardiola86 commentedComment #12
mukeshaddweb commentedIssue is:
ImageStyle::addExtension() appends the new extension rather than replacing the original, producing double-extension derivative filenames:
public://image.png → public://styles/medium/public/image.png.webp
// Current: $path .= '.' . $extension; // image.png → image.png.webp
Fix: $path = substr($path, 0, -(strlen($original_extension) + 1)) . '.' . $extension;
replaces the extension instead of appending it, produce clean single-extension filenames.
Comment #13
mukeshaddweb commentedComment #14
smustgrave commentedHave not reviewed but fixes need to be in MRs please