WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 40 hours 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: has-patch
Focuses: Cc:

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().

Attachments (1)

48427.diff (825 bytes) - added by donmhico 3 weeks ago.

Download all attachments as: .zip

Change History (8)

#1 @pbiron
4 months ago

  • Description modified (diff)

#2 @davidbaumwald
4 months 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
4 months 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
4 months 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.

@donmhico
3 weeks ago

#5 @donmhico
3 weeks ago

  • Keywords has-patch added

Thanks for the report @pbiron. The patch 48427.diff removes the second } elseif ( is_post_type_archive() ) {.

#6 @pbiron
3 weeks ago

thanx @donmhico. looks good to me. @SergeyBiryukov?

#7 @audrasjb
40 hours ago

Hi there,

With Beta 3 approaching, we'll need a decision and a commit action very soon.
For Component maintainers: if you don't feel the current patch is ready to land in WP 5.4 in the next few day, it's could be better to move it to 5.5. We're leaving it in the milestone for now.

@SergeyBiryukov, are your available to review the current patch? If not, it may be better to moved it to 5.5 :-)

Note: See TracTickets for help on using tickets.