WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 years ago

#37255 reviewing enhancement

wp_get_attachment_caption and post parameter

Reported by: wido Owned by: joemcgill
Milestone: Future Release Priority: normal
Severity: normal Version: 4.6
Component: Media Keywords: needs-patch
Focuses: Cc:
PR Number:

Description

Some WordPress functions accept the $post object as parameter instead only the $post_id.

Since most of the time we write our code within the loop or we use the $post object, could be an improvement to standardize the functions that require a post id even support the $post object?

For the wp_get_attachment_caption for example, could be like this:

/**
 * Retrieves the caption for an attachment.
 *
 * @since 4.6.0
 *
 * @param int|WP_Post|null $post Optional. Post ID or post object. Defaults to global $post.
 * @return string|false False on failure. Attachment caption on success.
 */
function wp_get_attachment_caption( $post = null ) {

	if ( ! $post instanceof WP_Post ) {
		if ( ! $post = get_post( $post ) ) {
			return false;
		}
	}

	if ( 'attachment' !== $post->post_type ) {
		return false;
	}

	$caption = $post->post_excerpt;

	/**
	 * Filters the attachment caption.
	 *
	 * @since 4.6.0
	 *
	 * @param string $caption Caption for the given attachment.
	 * @param WP_Post   $post Attachment object.
	 */
	return apply_filters( 'wp_get_attachment_caption', $caption, $post );
}

The get_the_post_thumbnail_caption that use the wp_get_attachment_caption accept the $post object.

Also, instead of passing the $post->ID to the wp_get_attachment_caption, use the $post object, so we can work directly with the object instead of calling again the get_post function to retrieve it.

Change History (6)

#1 @joemcgill
3 years ago

  • Component changed from Posts, Post Types to Media
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.6
  • Owner set to joemcgill
  • Status changed from new to reviewing

Hi @wido,

Thanks for the suggestion. It may be to late to get this in 4.6, but since this is a brand new function, it's not really an enhancement as much as a possibly missed implementation detail, so let's take a look at it.

This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.


3 years ago

#3 follow-up: @flixos90
3 years ago

  • Milestone changed from 4.6 to Future Release

Most similar attachment functions only accept an ID, so I just moved that over to the new function for wp_get_attachment_caption(). It makes sense to improve them to accept not only an ID, but also a post object IMO.

However, although it is a straightforward fix, it will affect quite a few functions that have been around for a longer time. I wouldn't like only changing it for wp_get_attachment_caption() just because we can (since it's new). Because of this broader scope let's think about it in 4.7. There needs to be a discussion whether we should change it for all the attachment functions or leave it as is.

While it wouldn't hurt to support objects, I personally never felt somewhat limited by using IDs for attachments. Maybe it's just because it has already sunk in that attachment functions accept IDs.

Anyway, the functions that would need to be enhanced would be:

In wp-includes/post.php:

  • wp_get_attachment_metadata()
  • wp_update_attachment_metadata()
  • wp_get_attachment_url()
  • wp_get_attachment_caption()
  • wp_get_attachment_thumb_file()
  • wp_get_attachment_thumb_url()

In wp-includes/media.php:

  • image_downsize()
  • get_image_tag()
  • image_get_intermediate_size()
  • wp_get_attachment_image_src()
  • wp_get_attachment_image()
  • wp_get_attachment_image_url()
  • wp_get_attachment_image_srcset()
  • wp_get_attachment_image_sizes()

I might be missing some more. The only attachment function I found that supports passing an object is wp_attachment_is().

#4 in reply to: ↑ 3 ; follow-up: @wido
3 years ago

There are also these functions that support the WP_Post object as parameter:

post_password_required
setup_postdata
get_post
get_post_type
sanitize_post
wp_trash_post_comments
wp_untrash_post_comments
get_page_uri
get_oembed_response_data
get_page_templates

#5 in reply to: ↑ 4 @flixos90
3 years ago

Replying to wido:

There are also these functions that support the WP_Post object as parameter [...]

I think what we need to discuss here is whether attachment functions are intentionally different from regular post functions (like the ones you mentioned) or whether it just hasn't been improved yet.
I tend to agree with you that these functions should support post objects too, but due to the impact of this change, we probably won't be able to get it in this release cycle.

#6 @shulard
3 years ago

Hello !

I've just faced the issue when using wp_get_attachment_url function. I think that the behaviour of media functions must be normalized with others functions.

I started to work on a patch but it's bigger than expected and can't finish it until next week.

Relying on get_post will not break the current behaviour of the functions : retrieve post from ID when necessary, elsewhere use the already loaded data.

Note: See TracTickets for help on using tickets.