Opened 4 weeks ago
Last modified 4 weeks ago
#65853 new defect (bug)
Using is_feed() does not always work
| Reported by: | josephscott | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Awaiting Review |
| Component: | Feeds | Version: | |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: | performance |
Description
There are code paths where is_feed() is being used, but it does not do what was intended because the query hasn't been parsed yet. That puts is_feed() into a state where $wp_query exists, so _doing_it_wrong() does not get called, but it still incorrectly returns false for is_feed(). Because $wp_query->is_feed( $feeds ); defaults to false.
This can be an issue for things that run via init.
I've got a PR that will add a new function ( and tests ) to check if the request looks like a one for a feed. It will also replace a few is_feed() calls that were not working correctly.
Change History (5)
This ticket was mentioned in PR #12976 on WordPress/wordpress-develop by @josephscott.
4 weeks ago
#1
- Keywords has-patch has-unit-tests added
#2
in reply to: ↑ description
@
4 weeks ago
Replying to josephscott:
There are code paths where
is_feed()is being used, but it does not do what was intended because the query hasn't been parsed yet. That putsis_feed()into a state where$wp_queryexists, so_doing_it_wrong()does not get called, but it still incorrectly returnsfalseforis_feed(). Because$wp_query->is_feed( $feeds );defaults tofalse.
What about just modifying the _doing_it_wrong() code so that it fires if ! did_action( 'parse_query' ) in addition to when ! isset( $wp_query )?
#3
@
4 weeks ago
For a parallel, the wp_enqueue_script() function calls _wp_scripts_maybe_doing_it_wrong() which checks:
<?php if ( did_action( 'init' ) || did_action( 'wp_enqueue_scripts' ) || did_action( 'admin_enqueue_scripts' ) || did_action( 'login_enqueue_scripts' ) ) {
#4
@
4 weeks ago
WordPress has tried to avoid backwards compatibility issues, so changing the behavior of an existing function - one that is more than 20 years old - wasn't my first option. And none of the comments or documentation that I saw on it gave any indication that it only worked correctly after the query had been parsed. If there was something to indicate that is_feed() was only reliable after request parsing then I think there would be an argument for changing how it worked to comply with that description.
It certainly would work for the cases that I was looking at, but I don't know what assumptions themes and plugins have made about it. Anything that expects the existing behavior has the potential to break ( or at least get different results ) after a change like that.
Given the large usage of WordPress and how long this function has been around it runs the risk of hitting Hyrum's Law.
#5
@
4 weeks ago
The other angle is that triggering _doing_it_wrong() doesn't actually fix the issues with wp_should_load_separate_core_block_assets() and wp_should_load_block_assets_on_demand() - they still need a way to determine if WP is processing a feed request or not before the request parser has happened.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
https://core.trac.wordpress.org/ticket/65853
This adds the
wp_is_serving_feed_request()function, which can be used instead ofis_feed()in cases where the query parser has not run yet. Code that runs off of theinitaction falls into that category.Included are updates to
wp_should_load_separate_core_block_assets()andwp_should_load_block_assets_on_demand()to replace their use ofis_feed(). Because they were both usingis_feed()before the query was parsedis_feed()always returnedfalse. Which means the early return didn't actually happen.All this results in WordPress feed requests doing more work than they needed to. In my local testing this reduced TTFB for feed requests 39.3 ms to 37.8 ms at p75. That is a savings of ~1.5 ms or 3.9%.
See https://core.trac.wordpress.org/changeset/50836 for the original work on
should_load_separate_core_block_assets()- more than 5 years ago.AI assistance: Yes
Tool(s): Claude
Model(s): Fable 5
Used for: The code was a joint effort between myself and Claude. The tests were written by Claude.