![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
Opened 6 years ago
Last modified 3 days ago
#51117 assigned defect (bug)
Sitemap & XSL requests execute main query for home page.
| Reported by: | peterwilsoncc | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | Sitemaps | Version: | 5.5 |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: | performance |
Description (last modified by )
When requesting a sitemap, the main query is run with the parameters for the is_home() request by default. The same is true for the stylesheet.
- On a site without posts this can return a false 404 error for the sitemap
- On a site without a persistent cache, this can result in 10 unneeded database queries.
Database log for requesting the first page of the posts sitemap is attached, other sitemap requests look similar up until the sitemap specific DB query is reached,
Proposal:
- Prevent main query on sitemaps
- Move stylesheet to a hard coded xsl file if possible
Attachments (1)
Change History (21)
#1
@
6 years ago
- Description modified (diff)
- Summary Sitemap & XLS requests execute main query for home page. → Sitemap & XSL requests execute main query for home page.
#3
follow-up:
↓ 4
@
5 years ago
@pbiron I've added some notes to #51912 (as reported on a duplicate ticket) as this can cause false 404 status codes on valid sitemap pages.
I'll leave you to decide if this should be closed as a duplicate of the other ticket.
#4
in reply to: ↑ 3
@
5 years ago
Replying to peterwilsoncc:
@pbiron I've added some notes to #51912 (as reported on a duplicate ticket) as this can cause false 404 status codes on valid sitemap pages.
Thanx for the ping...I had forgotten about this ticket.
Not sure when I'll have time to look into this more, but I'll do my best to at least investigate how the REST API does the short circuit and see how easy it will be to at least do it for sitemaps (and leave robots and feeds for later).
#5
@
5 years ago
I've added a potential fix at https://core.trac.wordpress.org/ticket/51912#comment:7, if that's any help?
#6
@
4 years ago
This plugin should resolve the issue https://wordpress.org/plugins/xml-sitemaps-manager/ which applies the solution proposed by @Tkama here https://core.trac.wordpress.org/ticket/51912#comment:9
Please let me know if there are still problems after applying the fix.
#7
follow-up:
↓ 8
@
3 years ago
I expect class WP_Query to have an $is_sitemap property just like it has $is_robots and $is_favicon.
That parameter needs to be tested against before setting is_home to true in method WP_Query::parse_query().
With this ticket resolved, we can undo the patch [53548].
#8
in reply to: ↑ 7
@
3 years ago
- Keywords needs-patch added
Replying to Cybr:
I expect class
WP_Queryto have an$is_sitemapproperty just like it has$is_robotsand$is_favicon.
That parameter needs to be tested against before setting
is_hometotruein methodWP_Query::parse_query().
Makes sense to me 👍
#9
follow-up:
↓ 11
@
3 years ago
Currently using the following as a patch (via plugin) solving all issues. The functions is_sitemap() and is_sitemap_stylesheet() are not technically needed in this context but seem logical to have available (and use)...
Note that hooking wp_sitemaps_loaded() into parse_request will render the sitemap (or stylesheet) early and then exit, thereby skipping the unneeded main query altogether.
add_action( 'parse_request', 'wp_sitemaps_loaded' );
if ( ! function_exists( 'wp_sitemaps_loaded' ) ) :
/**
* Loads the WordPress XML Sitemap Server
*
* @see https://core.trac.wordpress.org/ticket/51912
*
* @since 1.0
*
* @param WP $wp Current WordPress environment instance.
* @global WP_Query $wp_query WordPress Query.
* @return void
*/
function wp_sitemaps_loaded( $wp ) {
global $wp_query;
/**
* Whether this is a Sitemap Request.
*
* @see https://core.trac.wordpress.org/ticket/51543
* @since 1.0
* @var bool
*/
$wp_query->is_sitemap = ! empty( $wp->query_vars['sitemap'] );
/**
* Whether this is a Sitemap Stylesheet Request.
*
* @since 1.0
* @var bool
*/
$wp_query->is_sitemap_stylesheet = ! empty( $wp->query_vars['sitemap-stylesheet'] );
if ( ! is_sitemap() && ! is_sitemap_stylesheet() ) {
return;
}
// Prepare query variables.
$query_vars = $wp_query->query_vars;
$wp_query->query_vars = $wp->query_vars;
// Render the sitemap.
wp_sitemaps_get_server()->render_sitemaps();
// Still here? Then it was an invalid sitemap request after all. Undo everything and carry on...
$wp_query->is_sitemap = false;
$wp_query->is_sitemap_stylesheet = false;
$wp_query->query_vars = $query_vars;
}
endif;
if ( ! function_exists( 'is_sitemap' ) ) :
/**
* Determines whether the query is for the sitemap.
*
* For more information on this and similar theme functions, check out
* the {@link https://developer.wordpress.org/themes/basics/conditional-tags/
* Conditional Tags} article in the Theme Developer Handbook.
*
* @see https://core.trac.wordpress.org/ticket/51543
*
* @since 1.0
*
* @global WP_Query $wp_query WordPress Query object.
* @return bool Whether the query is for the sitemap.
*/
function is_sitemap() {
global $wp_query;
if ( ! isset( $wp_query ) ) {
_doing_it_wrong( __FUNCTION__, translate( 'Conditional query tags do not work before the query is run. Before then, they always return false.' ), '3.1.0' );
return false;
}
return property_exists( $wp_query, 'is_sitemap' ) ? $wp_query->is_sitemap : false;
}
endif;
if ( ! function_exists( 'is_sitemap_stylesheet' ) ) :
/**
* Determines whether the query is for the sitemap stylesheet.
*
* For more information on this and similar theme functions, check out
* the {@link https://developer.wordpress.org/themes/basics/conditional-tags/
* Conditional Tags} article in the Theme Developer Handbook.
*
* @since 1.0
*
* @global WP_Query $wp_query WordPress Query object.
* @return bool Whether the query is for the sitemap stylesheet.
*/
function is_sitemap_stylesheet() {
global $wp_query;
if ( ! isset( $wp_query ) ) {
_doing_it_wrong( __FUNCTION__, translate( 'Conditional query tags do not work before the query is run. Before then, they always return false.' ), '3.1.0' );
return false;
}
return property_exists( $wp_query, 'is_sitemap_stylesheet' ) ? $wp_query->is_sitemap_stylesheet : false;
}
endif;
#10
@
5 months ago
I switched off the users sitemap via the 'wp_sitemaps_add_provider' filter and was expecting to get a 404 on /wp-sitemap-users-1.xml but got a list of blog posts with code 200. For /wp-sitemap-users-2.xml I got a 404 page, which isn't ideal also because it loads the whole system. I assume I will also get 200 on /wp-sitemap-users-1.xml if I have more than 10 posts.
#11
in reply to: ↑ 9
;
follow-up:
↓ 12
@
4 months ago
Replying to RavanH:
Currently using the following as a patch (via plugin) solving all issues. The functions
is_sitemap()andis_sitemap_stylesheet()are not technically needed in this context but seem logical to have available (and use)...
Note that hooking
wp_sitemaps_loaded()intoparse_requestwill render the sitemap (or stylesheet) early and then exit, thereby skipping the unneeded main query altogether.
<?php ...
Thank you! Could you push this code to core? The issue is 5 years old. It's time for somebody competent to do something about it. It's ridiculous how long this crucial issue persists unresolved.
#12
in reply to: ↑ 11
;
follow-up:
↓ 13
@
4 months ago
Replying to JayFry:
Replying to RavanH:
Currently using the following as a patch (via plugin) solving all issues. The functions
is_sitemap()andis_sitemap_stylesheet()are not technically needed in this context but seem logical to have available (and use)...
Note that hooking
wp_sitemaps_loaded()intoparse_requestwill render the sitemap (or stylesheet) early and then exit, thereby skipping the unneeded main query altogether.
<?php ...Thank you! Could you push this code to core? The issue is 5 years old. It's time for somebody competent to do something about it. It's ridiculous how long this crucial issue persists unresolved.
@JayFry this approach is implemented in https://wordpress.org/plugins/xml-sitemaps-manager/ and https://wordpress.org/plugins/xml-sitemap-feed/
#13
in reply to: ↑ 12
@
4 months ago
Replying to RavanH:
Replying to JayFry:
Replying to RavanH:
Currently using the following as a patch (via plugin) solving all issues. The functions
is_sitemap()andis_sitemap_stylesheet()are not technically needed in this context but seem logical to have available (and use)...
Note that hooking
wp_sitemaps_loaded()intoparse_requestwill render the sitemap (or stylesheet) early and then exit, thereby skipping the unneeded main query altogether.
<?php ...Thank you! Could you push this code to core? The issue is 5 years old. It's time for somebody competent to do something about it. It's ridiculous how long this crucial issue persists unresolved.
@JayFry this approach is implemented in https://wordpress.org/plugins/xml-sitemaps-manager/ and https://wordpress.org/plugins/xml-sitemap-feed/
With all due respect to your plugins, this is something that needs to be fixed in core.
This ticket was mentioned in PR #12314 on WordPress/wordpress-develop by @sukhendu2002.
3 weeks ago
#14
- Keywords has-patch has-unit-tests added; needs-patch removed
Trac ticket: https://core.trac.wordpress.org/ticket/51117
#20
@
9 days ago
- Milestone Awaiting Review → 7.1
- Owner changed from to
- Status accepted → assigned
Sorry for the duplicate noise. There is an internal error in Trac preventing me from properly closing #51542 as a duplicate of this ticket.
This ticket will be fixed as part of https://github.com/WordPress/wordpress-develop/pull/12142 for #51543.
#22
@
9 days ago
- Milestone 7.1 → Future Release
- Owner removed
Sorry, just tested and #51542 is not a duplicate. The home query is still running needlessly.
#24
follow-ups:
↓ 25
↓ 26
@
9 days ago
I think we should remove the sitemap stylesheet from core. Chrome is deprecating and removing XSLT, and other browsers are doing the same.
#25
in reply to: ↑ 24
@
9 days ago
Replying to westonruter:
I think we should remove the sitemap stylesheet from core. Chrome is deprecating and removing XSLT, and other browsers are doing the same.
This news makes me very sad: XSLT is one of my favorite langs to write code in ;-(
I can hopefully re-write things to apply the XSLT transformation on the back end. Will have to will have to play with the rewrite rules to do that.
I can't promise when I'll get to it, but I will try to take an initial stab this weekend.
#26
in reply to: ↑ 24
@
3 days ago
Replying to westonruter:
I think we should remove the sitemap stylesheet from core. Chrome is deprecating and removing XSLT, and other browsers are doing the same.
@westonruter I opened #65635 for discussing (and work) on this.
I've noticed this as well and thought "why is it bothering to run the main query when the results of that aren't used?".
But I'd point out that sitemaps aren't the only "rewrite rule-only" request that does this:
https://example.com/robots.txtandhttps://example.com/feedrun the main query as well.While looking at this, I was happy to find that the REST API does not run the main query: it hooks into
parse_requestand short circuits the process when handling REST requests.I haven't looked in detail yet, but am pretty sure sitemaps, robots.txt and feeds could do the same thing. I wonder whether it would be beneficial to introduce a general mechanism that all such "rewrite rule-only" requests could build on?