Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Modern Image Format not cropping image if crop is an array #1887

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
+8 −8
Open

Fix Modern Image Format not cropping image if crop is an array #1887

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
83b0edf
Refactor image size handling to support additional crop data structure
b1ink0 Feb 26, 2025
af35deb
Simplify crop condition checks
b1ink0 Feb 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions plugins/webp-uploads/helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,11 @@ function webp_uploads_get_upload_image_mime_transforms(): array {
* @since 1.0.0
* @access private
*
* @param int $attachment_id The ID of the attachment from where this image would be created.
* @param string $image_size The size name that would be used to create the image source, out of the registered subsizes.
* @param array{ width: int, height: int, crop: bool } $size_data An array with the dimensions of the image: height, width and crop.
* @param string $mime The target mime in which the image should be created.
* @param string|null $destination_file_name The path where the file would be stored, including the extension. If null, `generate_filename` is used to create the destination file name.
* @param int $attachment_id The ID of the attachment from where this image would be created.
* @param string $image_size The size name that would be used to create the image source, out of the registered subsizes.
* @param array{ width: int, height: int, crop: bool|array{0: string, 1: string}} $size_data An array with the dimensions of the image: height, width and crop.
* @param string $mime The target mime in which the image should be created.
* @param string|null $destination_file_name The path where the file would be stored, including the extension. If null, `generate_filename` is used to create the destination file name.
*
* @return array{ file: string, filesize: int }|WP_Error An array with the file and filesize if the image was created correctly, otherwise a WP_Error.
*/
Expand All @@ -109,7 +109,7 @@ function webp_uploads_generate_additional_image_source( int $attachment_id, stri
* @param array{
* width: int,
* height: int,
* crop: bool
* crop: bool|array{0: string, 1: string}
* } $size_data An array with the dimensions of the image.
* @param string $mime The target mime in which the image should be created.
*/
Expand Down Expand Up @@ -156,7 +156,7 @@ function webp_uploads_generate_additional_image_source( int $attachment_id, stri

$height = isset( $size_data['height'] ) ? (int) $size_data['height'] : 0;
$width = isset( $size_data['width'] ) ? (int) $size_data['width'] : 0;
$crop = isset( $size_data['crop'] ) && $size_data['crop'];
$crop = isset( $size_data['crop'] ) ? $size_data['crop'] : false;
if ( $width <= 0 && $height <= 0 ) {
return new WP_Error( 'image_wrong_dimensions', __( 'At least one of the dimensions must be a positive number.', 'webp-uploads' ) );
}
Expand Down Expand Up @@ -240,8 +240,21 @@ function webp_uploads_generate_image_size( int $attachment_id, string $size, str
$size_data['height'] = $metadata['sizes'][ $size ]['height'];
}

if ( isset( $sizes[ $size ]['crop'] ) ) {
$size_data['crop'] = (bool) $sizes[ $size ]['crop'];
if (
isset( $sizes[ $size ]['crop'] ) &&
(
(
is_array( $sizes[ $size ]['crop'] ) &&
count( $sizes[ $size ]['crop'] ) === 2 &&
isset( $sizes[ $size ]['crop'][0] ) &&
isset( $sizes[ $size ]['crop'][1] ) &&
is_string( $sizes[ $size ]['crop'][0] ) &&
is_string( $sizes[ $size ]['crop'][1] )
) ||
is_bool( $sizes[ $size ]['crop'] )
)
) {
$size_data['crop'] = $sizes[ $size ]['crop'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to do all this granular validation on the crop key. WordPress Core doesn't do it and lets everything through. It only ensures at the very last point when it's actually used (in the image_resize_dimensions() function) that it has the correct format.

I think allowing any value here to pass through makes more sense as it makes our code aligned with Core and more future-proof, in case Core would later add support for other types for the crop key. It's also in line with width and height above, where we don't verify the type.

Suggested change
if (
isset( $sizes[ $size ]['crop'] ) &&
(
(
is_array( $sizes[ $size ]['crop'] ) &&
count( $sizes[ $size ]['crop'] ) === 2 &&
isset( $sizes[ $size ]['crop'][0] ) &&
isset( $sizes[ $size ]['crop'][1] ) &&
is_string( $sizes[ $size ]['crop'][0] ) &&
is_string( $sizes[ $size ]['crop'][1] )
) ||
is_bool( $sizes[ $size ]['crop'] )
)
) {
$size_data['crop'] = $sizes[ $size ]['crop'];
if ( isset( $sizes[ $size ]['crop'] ) ) {
$size_data['crop'] = $sizes[ $size ]['crop'];

Copy link
Contributor Author

@b1ink0 b1ink0 Feb 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added granular checks to ensure that the Modern Image Formats code does not produce errors on invalid input. However, to future-proof code, as you suggested, simplifying the condition makes more sense. Fixed in af35deb.

}

return webp_uploads_generate_additional_image_source( $attachment_id, $size, $size_data, $mime );
Expand Down
Loading