WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 4 months ago

#48427 closed defect (bug) (fixed)

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

Reported by: pbiron Owned by: johnbillion
Milestone: 5.4 Priority: normal
Severity: normal Version: 3.7
Component: Feeds Keywords: has-patch commit
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 5 months ago.

Download all attachments as: .zip

Change History (13)

#1 @pbiron
9 months ago

  • Description modified (diff)

#2 @davidbaumwald
9 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
9 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
9 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
5 months ago

#5 @donmhico
5 months ago

  • Keywords has-patch added

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

#6 @pbiron
5 months ago

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

#7 @audrasjb
5 months 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 :-)

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


5 months ago

#9 @SergeyBiryukov
5 months ago

  • Keywords commit added

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


4 months ago

#11 @johnbillion
4 months ago

  • Owner changed from SergeyBiryukov to johnbillion

#12 @johnbillion
4 months ago

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

In 47406:

Feeds: Remove an unreachable condition when generating archive feed links.

Props donmhico, pbiron

Fixes #48427

Note: See TracTickets for help on using tickets.