Make WordPress Core

Opened 13 years ago

Closed 11 years ago

#19412 closed defect (bug) (wontfix)

Changeset 18995 can lead to Notice: Undefined index: wp_the_query

Reported by: bobbingwide's profile bobbingwide Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.3
Component: Query Keywords: has-patch close
Focuses: Cc:

Description (last modified by ryan)

In changeset r18995 there were two changes to query.php
The comment "new does not require by reference" applies to the first but not the second change.

Should the assignment by reference in wp_reset_query have been removed?

The reported problem is:
http://wordpress.org/support/topic/plugin-wordpress-seo-by-yoast-notice-undefined-index-wp_the_query?replies=2#post-2477593

I had the same problem and questioned the suggested fix.
What I don't understand is why there is no Notice: message for the original code.

Try running this code.

<?php
error_reporting( E_ALL );  
@ini_set('display_errors',1);

function bad() {
  print_r( __FUNCTION__ );
  echo "\n";

  $GLOBALS = array( 'wp_query' => 'set' );
  print_r( $GLOBALS );
  unset($GLOBALS['wp_query']);
  $GLOBALS['wp_query'] = $GLOBALS['wp_the_query'];
  print_r( $GLOBALS );
  $GLOBALS['wp_query'] =& $GLOBALS['wp_the_query'];
  print_r( $GLOBALS );
}

function good() {
  print_r( __FUNCTION__ );
  echo "\n";
  $GLOBALS = array( 'wp_query' => 'set' );
  print_r( $GLOBALS );
  unset($GLOBALS['wp_query']);
  $GLOBALS['wp_query'] =& $GLOBALS['wp_the_query'];
  print_r( $GLOBALS );
  $GLOBALS['wp_query'] = $GLOBALS['wp_the_query'];
  print_r( $GLOBALS );
}

good();
bad();

Attachments (1)

19412.patch (538 bytes) - added by SergeyBiryukov 13 years ago.

Download all attachments as: .zip

Change History (15)

#1 @SergeyBiryukov
13 years ago

19412.patch adds a _doing_it_wrong() message if wp_reset_query() was called before query_posts().

#2 @SergeyBiryukov
13 years ago

  • Keywords has-patch added

#3 @ryan
13 years ago

  • Description modified (diff)

#4 @aaroncampbell
13 years ago

  • Milestone changed from Awaiting Review to 3.3

#5 @nacin
13 years ago

  • Keywords close added

Per a discussion in IRC earlier today, I think it would be better to just close this as wontfix.

  • If $wp_the_query is an object, then everything will work.
  • If $wp_the_query is not yet set, then nothing will break (correct me if I'm wrong) and a notice will result.

At this point, if you're using it wrong, you get a notice. Seems fine to me. The alternatives are A) return silently if $wp_the_query isn't set, or B) issue a _doing_it_wrong() notice. B) is kind of redundant since a notice is already being returned, and A) hides from developers that there is a problem.

Has this been seen outside of Yoast's plugin? I imagine he'll be quick to fix it.

#6 @ryan
13 years ago

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

Agreed.

#7 @SergeyBiryukov
13 years ago

  • Milestone 3.3 deleted

#8 follow-up: @bobbingwide
13 years ago

I didn't see the IRC discussion but I would have opted for alternative A.
IMHO it's better defensive coding.
Who says there's a problem if wp_reset_query is called before query_posts().

And it still doesn't explain why PHP masked the Notice when the & was present.


#9 @bobbingwide
13 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

May I refer you to this blog post

http://nacin.com/2010/04/23/5-ways-to-debug-wordpress/

it contains
WP_DEBUG will often reveal potential problems in your code, such as unchecked indexes (empty() and isset() are your friend) and undefined variables. (You may even find problems in WordPress itself, in which case you should file a bug report.)

#10 in reply to: ↑ 8 @SergeyBiryukov
13 years ago

  • Milestone set to Awaiting Review

#11 @bobbingwide
13 years ago

Thanks I just read the discussion as I was writing something about the shortcodes and wpautop problem #10457

PS. Shouldn't there be a similar change in wp-settings.php?
$wp_query =& $wp_the_query;

Last edited 13 years ago by bobbingwide (previous) (diff)

#12 @nacin
13 years ago

Replying to bobbingwide:

And it still doesn't explain why PHP masked the Notice when the & was present.

Because it creates a reference between variables, rather than any sort of assignment. So you can say $a =& $b; without $b being set, because then $b references $a (or vice versa). However, objects are automatically passed by reference, which means that if $b is an object, then $a = $b; makes them both reference the same object.

Where the notice kicks in is where $wp_the_query isn't yet defined. There is no requirement for query_posts() to have been called — the issue is specifically calling a query-related function before the query is even initialized in wp-settings.php.

(You may even find problems in WordPress itself, in which case you should file a bug report.)

Thank you for referring me to one of my own blog posts. :-)

The change in wp-settings.php could also occur.

#13 @scribu
12 years ago

Related: #22125

#14 @wonderboymusic
11 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

For that lingering ref, fixed here: [22434]

Note: See TracTickets for help on using tickets.