#9256 closed enhancement (fixed)
clean up the global variables when moving out of the loop
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.1 | Priority: | normal |
Severity: | normal | Version: | 2.7.1 |
Component: | Query | Keywords: | has-patch |
Focuses: | Cc: |
Description
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 (4)
Change History (25)
#2
@
16 years ago
- Keywords reporter-feedback added; sidebar the_content removed
- Milestone changed from 2.8 to Unassigned
#3
@
16 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.
#4
@
16 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
#6
@
16 years ago
- Component changed from General to Query
- Keywords has-patch needs-testing added; needs-patch removed
in case it's still useful.
#9
@
15 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.
#11
@
15 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. */ endwhile; $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).
#13
@
12 years 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:
- You have a paginated post and you are on page 2
- You make your own query for other posts anywhere else.
- You call
setup_postdata()
on a post with 2 pages - You call
the_content()
- Page 2's content shows up for that post
9256.2.diff fixes that.
#15
follow-up:
↓ 17
@
11 years 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.
#17
in reply to:
↑ 15
@
11 years 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.
#18
@
10 years ago
- Keywords needs-unit-tests removed
- Milestone changed from Future Release to 4.1
This ticket is essentially the same as #20904 and #25349. The root issue is that setup_postdata()
is not always sensitive to its current environment (ie, the WP_Query
class from which it's initiated). 9256.4.patch addresses this root issue by doing the following:
- Convert
setup_postdata()
to a method ofWP_Query
, and makesetup_postdata()
a wrapper for$wp_query->setup_postdata
for backward compatibility. (see #20904) - In
WP_Query::setup_postdata()
, get the value of$page
out of the current object, rather than usingget_query_var()
, which references the$wp_query
global. This means that<!--nextpage-->
is respected in secondary loops. - In
WP_Query::setup_postdata()
, only force$more
to 1 when (a) you're looking at an RSS feed, or (b) you're looking at the permalink of the$post
passed into the method. This means that<!--more-->
is respected in secondary loops. This change, along with the previous one, mean that (a)get_the_excerpt()
and (b)wp_link_pages()
work as expected in secondary loops. - Lots of unit tests, mainly to cover the more mundane parts of
setup_postdata()
. The critical ones for this ticket are the ones marked with@ticket
annotations.
I'm not sure if this counts as the "major cleanup" that nacin discusses earlier, but it's a substantial enough improvement that I think we should consider it for 4.1. Would like feedback from a lead on this one.
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.