Automattic / Automattic/wpcom-thumbnail-editor
Only first declared size for each aspect ratio is supported
- Dominant language
- PHP
- Stars
- 10
- Forks
- 11
- PR merge metrics
- No merged PRs in 30d
Description
The doc block for `get_image_sizes_by_ratio()` specifies that the method "Gets the first size defined for each dimension." This makes some sense if all sizes are cropped alike for a given aspect ratio, but it does mean that passing any declared image size _other than_ the first in the `image_ratio_map` list causes an error because of [these lines](https://github.com/Automattic/wpcom-thumbnail-editor/blob/master/wpcom-thumbnail-editor.php#L499-L500):
```
if ( empty( $_REQUEST['size'] ) || ! in_array( $_REQUEST['size'], $this->get_image_sizes_by_ratio() ) )
wp_die( sprintf( __( 'Invalid %s parameter.', 'wpcom-thumbnail-editor' ), 'size' ) );
```
If this is expected behavior, the documentation should be improved to explain that regardless of the size in an aspect ratio that we want to edit, we have to pass the first declared image size in `$this->image_ratio_map[$ratio]`.
If you should be able to pass another image size name that is present in `$this->image_ratio_map[$ratio]` other than `$this->image_ratio_map[$ratio][0]`, then the `get_image_sizes_by_ratio` method should be amended to return a more comprehensive list, or else the `$_REQUEST['size']` check should be changed to validate against the full list of supported names for a given ratio.
Contributor guide
No contributing guide indexed for this repository
Research direction
Read wpcom-thumbnail-editor.php around lines 499-500, then trace get_image_sizes_by_ratio() and image_ratio_map to compare the accepted names with the declared sizes. Confirm whether non-first sizes should be valid; done means either validation accepts the supported names or the documentation clearly states the first-size restriction.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100