#48304 closed defect (bug) (fixed)
Big Image: Revisit 'threshold' suffix on file names
Reported by: | kraftbj | Owned by: | azaozz |
---|---|---|---|
Milestone: | 5.3 | Priority: | normal |
Severity: | normal | Version: | 5.3 |
Component: | Media | Keywords: | has-patch |
Focuses: | Cc: |
Description
After #46076, when uploading a big image (over 2560px by default), there is special processing to create a new "full" size image that acts as the largest size that should usually be used by WordPress.
The naming convention employed by 46076 for this reduced image is the value of the threshold alone, e.g. image-2560.png
for a file of image.png
uploaded.
WordPress, previously, used both the height and width for all created sizes, e.g. image-2560x2000.png
.
Whenever a plugin/service is looking for the original image, it can commonly look to strip the suffix. For this new size, the suffix style has changed resulting in cases where it may not be able to realize it is not the original image.
Question: Are we open to not having a special naming convention for the threshold-limited file and let WordPress give it the standard image-2560x2000.jpg
file name?
Attaching a patch if we want to use the more traditional naming scheme and marking as 5.3 to ensure review before the new functionality ships as stable.
cc: @azaozz
Attachments (2)
Change History (18)
#2
follow-up:
↓ 4
@
5 years ago
I'll add if it is intentional to use a different format than an usual resize (maybe we don't want to make it that easy to use the original?), instead of -####.ext
, maybe something like -wpthres-####.ext
etc to make it more obvious where the file name came from while providing a pseudo-namespace.
This ticket was mentioned in Slack in #core-media by mike. View the logs.
5 years ago
#4
in reply to:
↑ 2
@
5 years ago
Replying to kraftbj:
I'll add if it is intentional to use a different format than an usual resize (maybe we don't want to make it that easy to use the original?)...
This is exactly the intention. Similarly to #48302, the scaled image is intended to replace the actual "attachment image" in all use cases, same as if the user has scaled the image before uploading it. The original image is kept in order to make better quality sub-sizes in the future.
...instead of
-####.ext
, maybe something like-wpthres-####.ext
etc to make it more obvious where the file name came from while providing a pseudo-namespace.
I'm open to changing the file name but it should probably be more descriptive and at the same time quite short. Having the "threshold" value in the name makes sense. Can possibly add wp-
in front or perhaps scaled-
?
On the other hand appending just the scale size seems to serve that purpose too?
#5
@
5 years ago
- Milestone changed from 5.3 to 5.4
Unfortunately this ticket was opened only few hours before 5.3-RC1. Going to move it to the 5.4 milestone for now but it can still be considered for 5.3 if a compelling user case exists.
#6
follow-up:
↓ 8
@
5 years ago
After discussing this a bit, the technical use case I have is less compelling. The compelling situation I have now that I would suggest 5.3 consideration is for support, developers, etc to be able to more easily identified that an image was scaled or threshold-bound by WP.
For support techs, seeing image-2560.png
won't be as obvious as image-scaled-2560.png
or something like that for when users/customers have questions about image sizing.
This ticket was mentioned in Slack in #core-media by mike. View the logs.
5 years ago
#8
in reply to:
↑ 6
@
5 years ago
Replying to kraftbj:
For support techs, seeing
image-2560.png
won't be as obvious asimage-scaled-2560.png
or something like that for when users/customers have questions about image sizing.
Yes, makes sense. As this is just a name change, thinking it's acceptable to do during RC. Would still need another committer to sign off.
In 48304.2.diff: append -scaled
plus the threshold size to scaled images file names.
#9
@
5 years ago
- Keywords 2nd-opinion removed
- Milestone changed from 5.4 to 5.3
Moving back to 5.3 for consideration.
This ticket was mentioned in Slack in #core by david.baumwald. View the logs.
5 years ago
#11
follow-ups:
↓ 12
↓ 13
@
5 years ago
For whatever it's worth, I want to note that to me the simpler format here is a major advantage of this approach as it allows us to construct the URL to the "threshold" version from any size, without knowing exact dimensions. We were previously filtering core in gross/scary ways to get this behavior, and it's been amazing to be able to revert this code and use a simpler approach in core.
Frankly, I'd like to see this change rolled out across the board; using the image size name as the suffix instead of exact dimensions. I'd be very disappointed to see this go away.
Alternately, it would be nice to see this be filterable; as noted above we were doing very gross/scary hacks to be able to customize the saved file name. If there were a filter in place to be able to easily filter the generated file name, that would more-than alleviate my concerns.
#12
in reply to:
↑ 11
@
5 years ago
Replying to chrisvanpatten:
For whatever it's worth, I want to note that to me the simpler format here is a major advantage of this approach as it allows us to construct the URL to the "threshold" version from any size, without knowing exact dimensions. We were previously filtering core in gross/scary ways to get this behavior, and it's been amazing to be able to revert this code and use a simpler approach in core.
Frankly, I'd like to see this change rolled out across the board; using the image size name as the suffix instead of exact dimensions. I'd be very disappointed to see this go away.
Not saying you're use case is unreasonable @chrisvanpatten, but wanted to play devil's advocate. Couldn't custom code just use Core's big_image_size_threshold
filter to construct the image name? Something like:
$size = apply_filters( 'big_image_size_threshold', 2560 ); $size_name = 'scaled-' . (int) $size;
Also, if the size is not found in the scaled image's name and a site adds a filter to big_image_size_threshold
to change the scaled value, it could become difficult to know which images have been scaled to the correct dimensions and which have not.
Alternately, it would be nice to see this be filterable;
A filter did come to mind here when looking through the changes.
As far as 48304.2.diff goes, I would be +1 to this to make it easier to identified images that were scaled or threshold-bound by WP.
#13
in reply to:
↑ 11
@
5 years ago
Replying to chrisvanpatten:
I want to note that to me the simpler format here is a major advantage of this approach as it allows us to construct the URL to the "threshold" version from any size, without knowing exact dimensions.
...
Frankly, I'd like to see this change rolled out across the board; using the image size name as the suffix instead of exact dimensions. I'd be very disappointed to see this go away.
As @desrosj points out above 48304.2.diff doesn't change that. It is to just append another (static) string before the threshold value to make these scaled images easier to recognize when looking at the uploads directory directly.
#14
@
5 years ago
- Owner set to azaozz
- Resolution set to fixed
- Status changed from new to closed
In 46565:
#15
@
5 years ago
A filter did come to mind here when looking through the changes.
To me this is the ultimate problem: right now, $editor->generate_filename
is not filterable, and I really feel that it should be. Otherwise, you have to resort to ugly hacks where you watch for core's generating of the files and need to rename the file in place. It would make it radically easier to apply this type of file structure (filename-size.ext
) across the board, rather than rely on the unpredictable image names that contain the image sizes.
I'm still pretty disappointed about this, but ultimately recognise why it was made. Ultimately it just means I have to re-enable my ugly hack code to rename files again :)
I agree it'd be better to have the naming be more consistent, if it's possible.
@azaozz is there a technical reason I'm forgetting/missing for choosing to name the size this way?