WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#21648 closed defect (bug) (fixed)

Enable feed autodiscovery for custom post type archives

Reported by: mdgl Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.4.1
Component: Posts, Post Types Keywords: has-patch
Focuses: Cc:

Description

When creating a new custom post type with register_post_type() it is possible to enable the automatic creation of archives and the corresponding feeds.

Unfortunately, such feeds (if enabled) are not advertised through the "feed autodiscovery" mechanism. That is, if you view a custom post type archive page, there is no <link> added to point to the corresponding feed. This does happen, for example with other archive pages such as those for search, author, date, category and tag.

To me, it looks like there is some missing code in feed_links_extra() [file: general_template.php] that would check is_post_type_archive() and output the appropriate link using get_post_type_archive_feed_link() and post_type_archive_title().

Attachments (4)

21648.diff (1020 bytes) - added by mdgl 6 years ago.
Suggested patch, although we could argue about the content of the title itself (eg. should it include "Post Type") and I'm not completely sure whether the use of get_post_type() here is correct.
21648-refresh.diff (1.2 KB) - added by wonderboymusic 5 years ago.
21648.2.diff (663 bytes) - added by MikeHansenMe 5 years ago.
update based on comment 8
21648.3.diff (625 bytes) - added by wonderboymusic 5 years ago.

Download all attachments as: .zip

Change History (16)

@mdgl
6 years ago

Suggested patch, although we could argue about the content of the title itself (eg. should it include "Post Type") and I'm not completely sure whether the use of get_post_type() here is correct.

#1 @nacin
5 years ago

  • Component changed from Feeds to Post Types
  • Keywords needs-refresh added
  • Milestone changed from Awaiting Review to 3.5

Looks good to me. "Post Type Feed" should probably become just "Feed". Then it'd be "Books Feed" or whatever.

#2 @wonderboymusic
5 years ago

  • Keywords has-patch added

#3 @wonderboymusic
5 years ago

  • Keywords needs-refresh removed

Refresh patch added

#4 @nacin
5 years ago

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

In [21984]:

Auto feeds for post type archives. props mgdl, wonderboymusic. fixes #21648.

#5 @mdgl
5 years ago

Sorry, but I noticed a couple of issues with this in further testing:

1) As I suspected, the use of get_post_type() fails with empty post type archives (because the global $post is not set). In this case, we output an invalid link (because of issue 2 below). To find the post type, we need to go via the queried object instead (all a bit messy!).

2) The two uses of isset() later in function feed_links_extra() don't work as expected because functions such as get_post_type_archive_feed_link() can return false rather than null. I think these two calls should be replaced with !empty().

#6 @SergeyBiryukov
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#7 @nacin
5 years ago

  • Type changed from enhancement to defect (bug)

Moving to bug, now.

#8 @mdgl
5 years ago

Yeah, this code is designed to be run during wp_head so should not rely on there being anything meaningful in the $post global (a common failing, however). A possible fix is to replace the following line in the patch:

   $href = get_post_type_archive_feed_link( get_post_type() );

with:

   $post_type_obj = get_queried_object();
   $href = get_post_type_archive_feed_link( $post_type_obj->name );

This is slightly fragile, but a similar approach is taken elsewhere in the codebase and I don't really want to open up the can of worms which is the nature and type of the queried object in the context of complex queries :-)

You could argue it would be nicer for the two "get post type archive [feed] link" functions to default to the current post type automatically, but that would require changes in link-template.php also.

Replacing the two subsequent calls to isset() with !empty() is not strictly necessary but would add further bulletproofing.

#9 @navjotjsingh
5 years ago

  • Cc navjotjsingh@… added

@MikeHansenMe
5 years ago

update based on comment 8

#10 @MikeHansenMe
5 years ago

  • Cc mdhansen@… added

#11 @wonderboymusic
5 years ago

Refreshed to use queried object

#12 @nacin
5 years ago

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

In 22483:

Consult the queried object for the post type. props mdgl, MikeHansenMe, wonderboymusic. fixes #21648.

Note: See TracTickets for help on using tickets.