Add ability to easily get path to sized images (such as parameter to get_attached_file() or wp_get_attachment_thumb_file())

[paulschreiber]

Currently, you can call get_attached_file() and get the path to the full-sized version of the image.

However, if you want the path to a smaller variant of the image, you end up with something like this:

<?php
$imagedata = wp_get_attachment_metadata( $image_id );
$file_path = get_attached_file( $image_id );
$thumbnail_file = str_replace( wp_basename( $file_path ), $imagedata['sizes'][ $size ]['file'], $file_path );

Or like this:

<?php
$imagedata = image_get_intermediate_size( $image_id, $size );
$upload_directory = wp_get_upload_dir();
$thumbnail_file = $upload_directory['basedir'] . '/' . $imagedata['path'];

For brevity, these examples omit the null dereference checks, which triple the size of the code needed.

This is similar to the behaviour of wp_get_attachment_thumb_file(), but that function hard-codes thumb as the only size.

It would be very helpful if this existed:

<?php
get_attached_file( int $attachment_id, bool $unfiltered = false, string $size = false )

Or this existed:

<?php
wp_get_attachment_thumb_file( int $post_id, string $size = false )

See related tickets #33959 (from 2015) and #17262 (from 2011).

I'm happy to make a patch for either/both of these approaches, but don't know which one you prefer, and don't want this to languish for 11 years.

[antpb]

Moving this to 6.2 so we can investigate a new function for paths that meets the intent of the suggestions of this ticket.

[audrasjb]

FWIW, and maybe I’m wrong, but the first proposal looks better to me as it is less opinionated (the second proposal contains thumb which can refer to the thumbnail size).

Trac ticket: https://core.trac.wordpress.org/ticket/51780

Currently, you can call get_attached_file() and get the path to the full-sized version of the image.
However, if you can't directly get the path to a smaller variant of the image, making further code implementation prone to error and not efficient.

[Mista-Flo]

Hey folks,

I came up with a patch for this ticket, I think it definitely makes sense to extends get_attached_file function with a size parameter, furthermore it doesn't affect any current usage of the function as the parameter will remain an empty string by default.

I also added a quick unit test to check the function works as expected, not sure if it needs further unit testing?

[joedolson]

Fixes $size param docs default value.

[joedolson]

PR looks good; updated the patch to correct the default value on the $size param in the documentation block.

Adds argument to get paths for intermediate sizes.

Trac ticket: https://core.trac.wordpress.org/ticket/51780

[joedolson]

In 55199:

Media: Add argument to get_attached_file() for subsizes.

Add a $size argument to get_attached_file() to simplify getting the path to an intermediate image size.

Props paulschreiber, audrasjb, Mista-Flo.
Fixes #51780.

Fixed in r55199

Fixed in r55199

[SergeyBiryukov]

In 55202:

Docs: Improve the DocBlock for get_attached_file().

Includes:

• Using 3-digit format for the @since tag.
• Minor wording updates and formatting corrections.

Follow-up to [3203], [4612], [6379], [8203], [24983], [29090], [55199].

See #51780, #56792.

[SergeyBiryukov]

Thanks for the commit!

Should the $size parameter be passed to the get_attached_file filter?

[Mista-Flo]

Replying to SergeyBiryukov:

Thanks for the commit!

Should the $size parameter be passed to the get_attached_file filter?

Indeed I missed that, I think it will be a nice addition if plugins need to do some actions on that filter.

[joedolson]

I agree; that makes a lot of sense. I think it would be confusing if it didn't.

[joedolson]

Passes $size to filter

Pass $size to filter.

Trac ticket: https://core.trac.wordpress.org/ticket/51780

[joedolson]

In 55217:

Media: Pass $size argument to get_attached_file filter.

Pass the new $size argument on get_attached_file() to get_attached_file filter. Follow up to [55199].

Props SergeyBiryukov, joedolson.
Fixes #51780.

Fixed in r55217

[azaozz]

Not sure this is a good addition to get_attached_file(). This function should get the attached file, i.e. a PDF, a video, a document, etc. This function is not intended only for images, what is the point to extend it only for image sub-sizes? That seems confusing and contrary to the initial design decision :(

Also, looking at the description, there is no reason why this is needed. If there is really a good reason for such function, it should probably be a new function.

However I don't see a reason why plugin authors cannot use image_get_intermediate_size() for this. That is probably currently being used. Getting the path to an image sub-size seems like something that's rarely needed anyway.

Another serious objection here is that this patch slows down get_attached_file() as it always does another DB request looking for image meta, no matter what file is attached.

In my opinion this patch should be reverted and moved to 6.3 or future release allowing for a better review whether this is really needed, and for better patch (new function) that doesn't reduce performance of get_attached_file()`.

[joedolson]

I'm not seeing how this impacts the performance of get_attached_file() by default; it only performs the additional request if the $size param isn't empty. Can you clarify that?

[azaozz]

Replying to joedolson:

it only performs the additional request if the $size param isn't empty

Hmm, true, but that's the expected use?

In any case, why a function that works for all attachments should be extended in a way that doesn't work for all?

[joedolson]

The logic, to me, is that images are the only file type that has managed variants, and it makes sense to use the same function to fetch variants of a file as you would use to fetch the attachment. The function is to get a file, not to get an attachment, so it makes sense that you could use it to fetch any file managed in the media library.

I could imagine it being extended to fetch alternate versions of audio/video files, too (e.g., different formats or different resolutions), though that isn't currently something managed by the library.

[azaozz]

Replying to joedolson:

it makes sense to use the same function to fetch variants of a file

The functionality of get_attached_file() is to "get the attached file", i.e. to get the path to the file that was uploaded by the user and is now "attached" to an attachment post.

The function is to get a file, not to get an attachment

Exactly, it is to get the path to the file attached to a particular attachment post.

fetch any file managed in the media library

I think I understand where you're coming from but don't think it is a good idea to extend a function that has to perform one simple task for all uploaded files, and make it work differently for different file types. In addition I don't think this is a good functionality to have in core anyway. It is not used anywhere in core, and there hasn't been a need for it for so many years.

On top of that the ticket description mentions that an existing function can be used for this, but doesn't mention why the proposed functionality would be useful. My guess is it would be a very minor convenience for maybe a few plugins that may meed the path to an image sub-size. Not sure this is a good reason to add this to core.

[paulschreiber]

On top of that the ticket description mentions that an existing function can be used for this, but doesn't mention why the proposed functionality would be useful. My guess is it would be a very minor convenience for maybe a few plugins that may meed the path to an image sub-size. Not sure this is a good reason to add this to core.

I gave context in the original ticket. Right now, everyone has to write 3 lines of code + null checks (so ≥12 lines). This greatly increases the possibility of error. Having core do this for you is very helpful, and core is more likely to get it right than random theme/plugin authors.

You can see the theme where this was used:
​https://github.com/CTCL/election-websites/blob/e5fc55dd651fb73d13114262b2dd60e8a095a91c/includes/class-helpers.php#L321-L356

Much of this was required because WPCOM behaves differently than everywhere else, which I detail in the comments in that source code.

[flixos90]

Apologies for chiming in late here, just came across this ticket. I agree with @azaozz that the idea of adding a $size parameter to this function conflicts with its purpose. This is a general attachment function, not a function for attachment images (or image attachments, whichever you wanna call it).

We have numerous functions specific to image attachments in core already, all of which (rightfully!) allow passing the $size parameter. But this here is not one of them. From the ticket description, I would think the wp_get_attachment_image_url() function is what should be used for this? But even if I'm missing something and maybe a function for something is indeed missing in core, we should rather add or modify an image attachment specific function, not this general one.

I think this changeset should be reverted. We can separately continue discussing for 6.3 whether the use-case here indeed requires a change, and which image function it would best fit into.

[joedolson]

wp_get_attachment_image_url() wouldn't do it, since it fetches URLs, rather than paths.

However, no problem with reverting this. I won't have time for it right away, so if anybody gets to it sooner, go right ahead.

[flixos90]

@joedolson Ah I see, this is about paths, that's what I had missed. :)

In that case I would rather suggest we introduce a new function for that purpose, e.g. wp_get_attachment_image_path( $attachment_id, $size ). That may be a useful enhancement for 6.3, and that way we build the image specific functionality into a new image specific function.

[azaozz]

Replying to paulschreiber:

Much of this was required because WPCOM behaves differently than everywhere else

Yes, the media on WP.com is handled in a quite different way. For example, afaik, there is no sizes array in the image meta, and image sub-sizes are created "on the fly" when needed and then cached for some time. If a convenience function for WP.com is needed, perhaps a better option would be to request it there.

Looking at the example use, it is for outputting a thumbnail image as data URL. Not sure if this is a common case to the extend that it would need a separate "convenience function", i.e. how many plugins would need that. But lets try to figure this out in 6.3.

[azaozz]

In 55437:

Media: Revert the addition of a $size parameter to get_attached_file().

Reverts [55199], [55202], and [55217] but keeps the updated docs.

Props: flixos90, joedolson, azaozz.
Fixes: #51780.

[flixos90]

After the revert, reopening this for continued discussion on whether a new image function should be introduced or an existing image function modified to support getting the full file path to a sub-sized image file.

[oglekler]

This is an enhancement, and we are in 12 days until Beta 1 after which we will not add new enhancement to the release, so, because there is no patch, I am moving this ticket to 6.4.

As far as I understand, the question now is if we need a new wp_get_attachment_image_path( $attachment_id, $size ) function or not, please continue discussion and look for use cases. From my point of view it is better to check that the file exists before using it for the frontend otherwise you can end with broken images, and it isn't looks nice. For example, new image size was added and used at some place but old images didn't regenerate. So, checking if image exists, we can use different size and avoid broken images to appear.

[oglekler]

This ticket was discussed during a bug scrub.

This ticket needs an analysis when image_get_intermediate_size() cannot fit the demand and a new function is needed.

Because this is an enhancement, please keep in mind that there is limited time to make it into 6.4 if some patch is needed.

Add props too: @hellofromTonya @vipuljnext

[azaozz]

As mentioned earlier I think the whole premise for this ticket is somewhat incorrect. The first line in the description says that:

Currently, you can call get_attached_file() and get the path to the full-sized version of the image.

Technically that is not exactly true. You get the the path to the attached file which may be an image.

I'm also not convinced that a separate helper function is needed to get paths to sub-size images. Currently this is easily done without a helper by using image_get_intermediate_size(). Even the name of that function suggests its usage.

After the revert, reopening this for continued discussion..

There hasn't been new info or opinions for six months. Re-closing this as fixed. Please feel free to reopen with more details why such helper function might be useful (although it seems a new enhancement ticket may be a better choice).