Make WordPress Core

Opened 5 years ago

Last modified 6 weeks ago

#9256 reopened enhancement

clean up the global variables when moving out of the loop

Reported by: aldolat Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.7.1
Component: Query Keywords: has-patch needs-unit-tests
Focuses: Cc:


On a theme, that I am developing, the sidebar has a new WP_Query to get only one post from a specific category. All works well (index, single, archive, pages) displaying correctly the result of the_content() of the main column (the main query) and the result of the_content() of my custom query in the sidebar.

The problem is that only on pages and articles that have pagination (using the <!--nextpage--> construct), the function "the_content()" of the sidebar displays the same content of the main column on pages 2, 3, 4 (and so on) of those pages/articles, while on page 1 the various contents are displayed properly.

Is it a bug?

As MichaelH reported, using "echo $post->post_content;" fixes the issue temporarily.

Attachments (3)

9256.diff (557 bytes) - added by Denis-de-Bernardy 5 years ago.
9256.2.diff (974 bytes) - added by wonderboymusic 8 months ago.
9256.3.diff (961 bytes) - added by wonderboymusic 3 months ago.

Download all attachments as: .zip

Change History (20)

comment:1 scribu5 years ago

  • Milestone changed from Unassigned to 2.8
  • Version set to 2.7.1

comment:2 Viper007Bond5 years ago

  • Keywords reporter-feedback added; sidebar the_content removed
  • Milestone changed from 2.8 to Unassigned

Are you making sure to use $query_string in your new query? Otherwise the URL parameters saying page 2 has been requested won't be passed.

comment:3 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added
  • Milestone changed from Unassigned to Future Release
  • Type changed from defect (bug) to enhancement

nah, it's related to the use of globals. even if you actually use the reset query functionality, WP will leave a boat load of globals around that then get used elsewhere.

comment:4 Denis-de-Bernardy5 years ago

  • Keywords reporter-feedback removed
  • Summary changed from Use of the_content() in sidebar to clean up the global variables when moving out of the loop

comment:5 Denis-de-Bernardy5 years ago

isn't there a wp_reset_query_vars() or something like that already?

comment:6 Denis-de-Bernardy5 years ago

  • Component changed from General to Query
  • Keywords has-patch needs-testing added; needs-patch removed

in case it's still useful.

Denis-de-Bernardy5 years ago

comment:7 ryan5 years ago

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

(In [11400]) Reset post-related globals in wp_reset_query(). Props Denis-de-Bernardy. fixes #9256

comment:8 ryan5 years ago

  • Milestone changed from Future Release to 2.8

comment:9 robbono5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I've applied and tested the attached patch to query.php, but using the modified wp_reset_query() function at the end of the single.php loop, for example, still does not prevent the post's content from being output when the_content() is used in the sidebar on the 2nd page of a paginated post.

comment:10 nacin4 years ago

  • Milestone changed from 2.8 to Future Release

comment:11 mdgl4 years ago

I think the problem here is in "setup_postdata()" (query.php) and its handling of global variables.

The function "setup_postdata()" sets the global variable "$page" based on the instance variables of the global query object, i.e. "$page = get_query_var('page')".

Of course if you have done a separate query (e.g. with "get_posts()", "get_children()" or a new "WP_Query" object) and are trying to make use of the template functions by calling "setup_postdata()", this is incorrect as the value of "$page" should be related to your separate query, not the enclosing loop.

The problem is further compounded because "setup_postdata()" only overwrites the first entry in the global "$pages[]" array if your separate query has only a single page. Hence, the other array entries still contain data from the main page and can be inadvertently accessed by function "get_the_content()" using the incorrect value of "$page".

I believe a workaround is to code as follows:

$myposts = &get_posts(...) /* Or new WP_Query or whatever. */
global $post, $page;
$mysavedpost = $post;
foreach ($myposts as $post) {
   setup_postdata($post); $page = 1;

   the_title(); the_content(); /* And so on. */

$post = $mysavedpost; setup_postdata($post);

With the increased use of widgets and complex page templates, such "nested loops" can only become more and more common. Although we're still using global variables in the loop, and "setup_postdata()" still needs to be fixed, a better solution here would be to offer full support for a "save/restore" paradigm for handling nested loops, for example:

$myposts = new WP_Query(...);
$myposts->save_loop_state(); /* Copies all required globals into new query object. */
while ($myposts->have_posts()) :
   $myposts->the_post(); /* Stomps on global variables. */
   ... /* Use global variables through template tags. */
$myposts->restore_loop_state(); /* Restores all required globals from new query object. */

With a bit of thought, this could perhaps even be made automatic and transparent within the query "have_posts()" and "the_post()" methods, although you would still have to have a method for coping with early exit from the loop.

Presently, we seem to have too much forcing of one loop to run after another, use of "wp_reset_query()" and so on, which is all a bit messy (IMHO).

comment:12 gvenk4 years ago

  • Cc gvenk added

wonderboymusic8 months ago

comment:13 wonderboymusic8 months ago

  • Milestone changed from Future Release to 3.7

This is very interesting one - to not break anything, I added an argument to setup_postdata() which will set the local var $page to 1 if there is no pagination requested:

function setup_postdata( $post, $paginate = true )

The gist of this ticket:

  1. You have a paginated post and you are on page 2
  2. You make your own query for other posts anywhere else.
  3. You call setup_postdata() on a post with 2 pages
  4. You call the_content()
  5. Page 2's content shows up for that post

9256.2.diff​ fixes that.

comment:14 wonderboymusic7 months ago

  • Keywords needs-unit-tests added; needs-testing removed

comment:15 follow-up: helen7 months ago

  • Milestone changed from 3.7 to Future Release

nacin: This whole globals mess needs a major cleanup. Way bigger than that. I suggest punt.

wonderboymusic3 months ago

comment:16 wonderboymusic3 months ago

  • Milestone changed from Future Release to 3.9

9256.3.diff is a refresh

comment:17 in reply to: ↑ 15 helen6 weeks ago

  • Milestone changed from 3.9 to Future Release

Replying to helen:

nacin: This whole globals mess needs a major cleanup. Way bigger than that. I suggest punt.

Seems like this still stands.

Note: See TracTickets for help on using tickets.