WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#20409 closed enhancement (fixed)

XML-RPC returns only full-size URL for featured image

Reported by: koke Owned by: ryan
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.4
Component: XML-RPC Keywords:
Focuses: Cc:

Description

I'm not sure if having more than one featured_image url would be too much for getPost (and similar calls), since there are also media library calls, but if there's going to be any, maybe the thumbnail size url would be more relevant

Attachments (5)

20409.patch (4.2 KB) - added by maxcutler 2 years ago.
20409_unittests.patch (1.1 KB) - added by maxcutler 2 years ago.
20409.2.patch (5.0 KB) - added by maxcutler 2 years ago.
20409_unittests.2.patch (3.2 KB) - added by maxcutler 2 years ago.
20409.diff (1.7 KB) - added by ryan 2 years ago.
Move functions

Download all attachments as: .zip

Change History (22)

comment:1 maxcutler2 years ago

  • Milestone changed from Awaiting Review to 3.4

comment:2 nacin2 years ago

Indeed, it would make more sense if only the thumbnail size was returned.

We don't yet have get_the_post_thumbnail_src() yet — #11571. But we can simply call wp_get_attachment_image() with $size = 'post-thumbnail'.

comment:3 markoheijnen2 years ago

  • Cc markoheijnen added

What about wp_get_attachment_thumb_url(). Seems like in this case that should work to.

That said wp_get_attachment_image() is a better way since it also can give the possibility to change the size when needed. Maybe not know but for future possibilities.

comment:4 maxcutler2 years ago

  • Cc maxcutler added

Another alternative is to extract most of wp_getMediaItem into a _prepare_media_item method and use that for featured_image in _prepare_post instead of an ID and a separate additional field.

That way you would have wp_getMediaItem, wp_getMediaLibrary, and _prepare_post all returning image/attachment data in an identical and filter-able format. And you'd solve the re-querying performance hit with wp_getMediaLibrary.

We are already doing this for terms in _prepare_post.

If others like this, I can draw up a patch and unit tests tonight.

comment:5 markoheijnen2 years ago

That does make sense to me.

comment:6 nacin2 years ago

Sure. It's worth mentioning that wp_get_attachment_thumb_url() uses a size of 'thumbnail' while post thumbnails are a particular 'post-thumbnail' size. So you still may need to use wp_get_attachment_image() with a $size.

maxcutler2 years ago

maxcutler2 years ago

comment:7 maxcutler2 years ago

Added a patch for my suggestion above. Now posts will have a post_thumbnail field which is an array containing the same values as would be gotten from wp.getMediaItem (or an empty array if there's no thumbnail).

As suggested by nacin, the thumbnail URL will use 'post-thumbnail' size if the theme supports it, and fall back to 'thumbnail' otherwise.

Also added basic tests in [UT693] to cover the existing wp.getMediaItem/wp.getMediaLibrary behavior so that we don't break that.

comment:8 ryan2 years ago

I think we can drop the extra underscore from the xmlrpcprepare_media_item filter.

Version 0, edited 2 years ago by ryan (next)

maxcutler2 years ago

comment:9 maxcutler2 years ago

Refreshed patch after comments in IRC:

  • Replaced get_post_meta calls with get_post_thumbnail_id
  • Removed 'wp_post_thumbnail_url' from the legacy methods.
  • Replaced one-line if statement with a ternary statement.

Will create another ticket for the extra underscores that ryan mentioned.

comment:10 ryan2 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [20608]:

XML-RPC featured image and media preparation cleanup.

  • Introduce _prepare_media_item().
  • Use it to prepare post_thumbnail info in _prepare_post().
  • Also use _prepare_media_item() in wp_getMediaLibrary() and wp_getMediaItem() so that all media is prepared consistently and uses the same filters.

Props maxcutler
Fixes #20409

comment:11 ryan2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

post-thumbnail-template.php is conditionally loaded based upon whether the theme supports post-thumbnails. If the theme does not, get_post_thumbnail_id() is undefined and results in fatal errors.

comment:12 ryan2 years ago

We can either always load post-thumbnail-template.php or move get_post_thumbnail_id() and perhaps has_post_thumbnail() to post.php.

comment:13 markoheijnen2 years ago

I would prefer moving get_post_thumbnail_id. Would make sense since updating and removing can be done when theme doesn't supports post-thumbnails

comment:14 ryan2 years ago

I'm leaning that way myself. Anyone want to patch it up and test? Perhaps we could cajole the unit tests into running with and without post-thumbnail support enabled.

ryan2 years ago

Move functions

comment:16 markjaquith2 years ago

post-thumbnail-template.php is so tiny... there honestly can't be any benefit to having it loaded conditionally. It's five function definitions.

comment:17 nacin2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [20610]:

Deprecate require_if_theme_supports(). Always require post-thumbnail-template.php. fixes #20556. fixes #20409.

Note: See TracTickets for help on using tickets.