WP_Query instances getting entangled with $wp_query global

[boonebgorges]

WP_Query::the_post() fires setup_postdata(), which then uses get_query_var(). get_query_var(), in turn, calls the get() method on the $wp_query global object. In this way, WP_Query instances are not truly insulated from each other - they all are tied up with $wp_query in a way that seems unnecessary, and causes fatal errors in some cases.

In normal use, you would never really notice the issue, because after the 'init' action, the following offending line in setup_postdata() will return the 'page' query var from $wp_query:

$page = get_query_var('page');

And, since secondary WP_Query instances don't paginate in the same way as the primary query, the 'page' property on those instances doesn't matter much. So, while the value of $page is not strictly speaking correct in these cases (it comes from the wrong object), it generally fails silently.

However, if you instantiate a WP_Query object before 'init', you get a fatal error, because $wp_query->get() is an undefined method. Here's a simple mu-plugin that will demonstrate the problem:

function bbg_create_early_wp_query_object() {
	$q = new WP_Query( array(
		'post_type' => 'post'
	) );

	if ( $q->have_posts() ) {
		while ( $q->have_posts() ) {
			$q->the_post();
		}
	}
}
add_action( 'plugins_loaded', 'bbg_create_early_wp_query_object' );

My suggested solution is to move the setup_postdata() logic into a method of the WP_Query class, and then use $this->get( 'page' ) rather than get_query_var( 'page' ), to ensure that you're referring to the query vars of the correct query object. The old function setup_postdata() then becomes a wrapper for $wp_query->setup_postdata().

In addition, I modified the way that the $more global is set, so that it references the current object as well. This prevents _doing_it_wrong() errors when firing up a WP_Query instance before 'init'.

[scribu]

We're still tied to all those globals, but this is definitely an improvement.

[wonderboymusic]

Boone, you are an attractive and intelligent man - I refreshed the patch so it doesn't blow up against trunk

[johnbillion]

This won't make 3.7 without unit tests. Punting.

[wonderboymusic]

20904.2.diff​ passes Unit Tests now, it was failing 3 of them

[nacin]

And, since secondary WP_Query instances don't paginate in the same way as the primary query, the 'page' property on those instances doesn't matter much.

Re-reading this, they don't matter much, but they do matter in some cases, right? Like if you're doing your own crazy handling? I am all for this change, though it seems minor, given it more or less works fine as-is. While it allows WP_Query to be used before init, I'm not sure allowing this is actually desirable. Post types aren't registered yet and a lot still needs to be initialized.

I'm wondering if we should tackle this in conjunction with #25349 and #9256. I'd be game for a 3.9 overhaul of post data setup, ideally untangling these globals in the process.

[boonebgorges]

While it allows WP_Query to be used before init, I'm not sure allowing this is actually desirable.

Fair enough, but the point about pre-init is just to give a particularly striking instance of the problem. After init, the leakiness is still there, but doesn't throw any notices, and so goes unnoticed.

given it more or less works fine as-is.

In WP, yes, but there are probably others that are attempting things that ought to be legit (however "crazy") but run into problems because the objects aren't encapsulated. 20904.2.diff is a very modest way of fixing it. If it could be rolled into a larger overhaul of the way post globals work, that'd be excellent, but if that doesn't end up happening in 3.9, I think this more conservative patch is worth considering.

[boonebgorges]

See https://core.trac.wordpress.org/ticket/9256#comment:18

[boonebgorges]

In 30085:

Improve global variable setting in setup_postdata().

setup_postdata() is responsible for setting a number of global variables
that are used for post pagination ($pages, $page, $nextpage) and the
generation of post excerpts ($more). These variables should be sensitive to
the currently running instance of WP_Query - rather than the main query -
so that these features work properly inside of secondary WP_Query loops.

This changeset moves the logic of setup_postdata() into a method on WP_Query,
and converts setup_postdata() to a wrapper.

Props boonebgorges, wonderboymusic.
See #25349.
Fixes #9256, #20904.