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 )
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)
Change History (13)
#3
@
9 months ago
- Milestone changed from Awaiting Review to 5.4
- Owner set to SergeyBiryukov
- Status changed from new to accepted
#4
@
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.
#5
@
5 months ago
- Keywords has-patch added
Thanks for the report @pbiron. The patch 48427.diff removes the second } elseif ( is_post_type_archive() ) {
.
#7
@
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 :-)
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.