WordPress.org

Make WordPress Core

Opened 6 weeks ago

Last modified 6 weeks ago

#48427 accepted defect (bug)

feed_links_extra() has 2 "elseif ( is_post_type_archive() ) {}" clauses

Reported by: pbiron Owned by: SergeyBiryukov
Milestone: 5.4 Priority: normal
Severity: normal Version: 3.7
Component: Feeds Keywords:
Focuses: Cc:
PR Number:

Description (last modified by pbiron)

While add some custom RSS feed support to a plugin, I discovered something odd about feed_links_extra().

It consists a large if/elseif/elseif statement and 2 of the elseif clauses are for is_post_type_archive().

<?php
if ( is_singular() ) {
...
} elseif ( is_post_type_archive() ) {
        $post_type = get_query_var( 'post_type' );
        if ( is_array( $post_type ) ) {
                $post_type = reset( $post_type );
        }

        $post_type_obj = get_post_type_object( $post_type );
        $title         = sprintf( $args['posttypetitle'], get_bloginfo( 'name' ), $args['separator'], $post_type_obj->labels->name );
        $href          = get_post_type_archive_feed_link( $post_type_obj->name );
}
...
} elseif ( is_post_type_archive() ) {
        $title         = sprintf( $args['posttypetitle'], get_bloginfo( 'name' ), $args['separator'], post_type_archive_title( '', false ) );
        $post_type_obj = get_queried_object();
        if ( $post_type_obj ) {
                $href = get_post_type_archive_feed_link( $post_type_obj->name );
        }
}

Obviously, the code in the 2nd one will never execute.

I'm a little unclear about how they both ended up in there, as Trac's blame is a little hard to follow in this case.

One of them should be removed...but not sure which one would be better to keep. Notice that the 1st one uses the post_type query arg in building the URL for the feed link, while the 2nd one uses get_queried_object().

Change History (4)

#1 @pbiron
6 weeks ago

  • Description modified (diff)

#2 @davidbaumwald
6 weeks ago

  • Version set to 3.7

Related #18614. Looks like the first } elseif ( is_post_type_archive() ) { was added in [25291] but the latter conditional already existed. Setting the version to 3.7 based on this.

#3 @SergeyBiryukov
6 weeks ago

  • Milestone changed from Awaiting Review to 5.4
  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

The second instance was added in [21984] for #21648, the first one in [25291] for #18614, when merging a patch created before [21984].

#4 @pbiron
6 weeks ago

Thanx both of you for tracking the history.

I think the 1st one (using get_query_var( 'post_type' ) is easier to understand. It takes a little bit of mental gymnastics to realize that get_queried_object() will return the post type object when is_post_type_archive() === true. Anyone disagree?

Just let me know which is the preferred one to keep and I'll create a patch for it.

Note: See TracTickets for help on using tickets.