WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#9854 closed defect (bug) (fixed)

wp_query gets poisoned by new WP_Query objects

Reported by: Denis-de-Bernardy Owned by: ryan
Milestone: 2.8 Priority: normal
Severity: normal Version: 2.8
Component: Query Keywords:
Focuses: Cc:

Description

See #7080 and #9458 for reference material.

Code to duplicate the issue:

while ( have_post() ) {
  the_post();
  the_title();
  the_widget('WP_Widget_Recent_Posts');
  the_content();
}

In the Recent Posts widget, add the following after the_post() to see things happening:

var_dump($GLOBALS['wp_the_query']->post->ID);

You'll notice is changes for no obvious reason.

Attachments (5)

9854.diff (1.7 KB) - added by jacobsantos 5 years ago.
Fix for Recent Posts widget.
9854.2.diff (1.3 KB) - added by Denis-de-Bernardy 5 years ago.
9854.3.diff (1.3 KB) - added by ryan 5 years ago.
9854-test.diff (2.2 KB) - added by Denis-de-Bernardy 5 years ago.
9854.4.diff (1.8 KB) - added by Denis-de-Bernardy 5 years ago.

Download all attachments as: .zip

Change History (34)

jacobsantos5 years ago

Fix for Recent Posts widget.

comment:1 jacobsantos5 years ago

  • Summary changed from wp_the_query gets poisoned by new WP_Query objects to wp_query gets poisoned by new WP_Query objects

comment:2 ryan5 years ago

Could we not use get_posts() here, do a regular foreach loop, and pass IDs/objects to the template functions?

comment:3 jacobsantos5 years ago

Ryan, thou be a genius.

comment:4 Denis-de-Bernardy5 years ago

we could, and it might fix it, but there would remain the underlying issue -- i.e. why is wp_the_query getting changed at all? ;-)

D.

comment:5 ryan5 years ago

(In [11382]) Break global post ref so we don't pollute queries. see #9854

comment:6 ryan5 years ago

WP::register_globals() does this:

$GLOBALS['post'] = & $wp_query->post;

Either removing that ref or breaking the ref in the_post() helps. Opted for ref break in [11382].

comment:7 Denis-de-Bernardy5 years ago

good catch. will give it a shot. say, isn't this more prudent?:

unset($GLOBALS['post']);
global $post;

comment:8 ryan5 years ago

Probably. Will redo.

comment:9 ryan5 years ago

(In [11386]) Unset from GLOBALS. see #9854

comment:10 jacobsantos5 years ago

I wonder, does this fix it?

comment:11 ryan5 years ago

I think it should. Just waiting confirmation.

comment:12 Denis-de-Bernardy5 years ago

Confirming it does, but I'd like to request an additional tweak in order to spare myself a few nightmares with a separate plugin. Patch coming up in a sec.

Denis-de-Bernardy5 years ago

comment:13 Denis-de-Bernardy5 years ago

the suggested additional patch is to allow plugins to decide whether to do anything on loop_start/loop_end based on whether it's the main loop or not.

comment:15 ryan5 years ago

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

(In [11399]) Pass query object to loop_start and loop_end actions. Props Denis-de-Bernardy. fixes #9854

comment:16 ryan5 years ago

Reports of breakage for loops like this:

$recent = new WP_Query("blah=blah");
while($recent->have_posts()) {
  $recent->the_post();
  echo $post->ID;
}

Evidently, $post is not being set.

comment:17 ryan5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:18 Denis-de-Bernardy5 years ago

on to it right away

comment:19 Denis-de-Bernardy5 years ago

mm, anything more specific? because this essentially is exactly what the recent posts widget does. :-P

comment:20 ryan5 years ago

Sent examples your way. Working on boiling it down to a test case.

comment:21 ryan5 years ago

The problem loop is in sidebar.php, which is loaded through get_sidebar() -> load_template(). load_template() sets up a $post global for use by sidebar.php, but we're stomping that global.

comment:22 ryan5 years ago

global $post creates a ref. We unset GLOBALSpost? in the_post(). The global $post used by sidebar.php now points at nothing.

comment:23 ryan5 years ago

Stick a loop in sidebar.php of the default theme to see:

$recent = new WP_Query('showposts=10');
while ( $recent->have_posts() ) {
	$recent->the_post();
	var_dump($post);
}

$post never changes

ryan5 years ago

comment:24 ryan5 years ago

Removing the ref from WP::register_globals() seems to fix everything. We can keep global $post as a ref for everyone to use without it messing with queries.

comment:25 Denis-de-Bernardy5 years ago

seems right. will give it a shot tomorrow.

comment:26 Denis-de-Bernardy5 years ago

first bit of the patch isn't necessarily useful though.

Denis-de-Bernardy5 years ago

Denis-de-Bernardy5 years ago

comment:27 Denis-de-Bernardy5 years ago

uploaded a diff for test cases and a slightly different patch (I moved the new the_post hook to a place where it's more useful on singular pages)

comment:28 Denis-de-Bernardy5 years ago

The other refs in WP::register_globals() might want the same kind of treatment at some point, too.

comment:29 ryan5 years ago

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

(In [11406]) Remove global regs that can poison query objects. fixes #9854

Note: See TracTickets for help on using tickets.