Opened 13 years ago
Last modified 10 months ago
#18408 assigned defect (bug)
Can't wp_reset_postdata after custom WP_Query in an admin edit page
Reported by: | ericlewis | Owned by: | audrasjb |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 4.7.2 |
Component: | Query | Keywords: | needs-testing has-patch early needs-unit-tests |
Focuses: | Cc: |
Description
While on an edit post page (or any post type), if you create a custom WP_Query object before the editor has been output, the post data from the custom WP_Query will fill out the edit post form, not the original content of the post that is actually trying to be edited.
I found this out when trying to create a custom metabox in the 'side' context. You can reproduce this by inserting this code into a plugin:
add_action( 'add_meta_boxes', 'myplugin_add_custom_box' ); function myplugin_add_custom_box() { add_meta_box( 'myplugin_sectionid', __( 'My Post Section Title', 'myplugin_textdomain' ), 'myplugin_inner_custom_box', 'post', 'side' ); } function myplugin_inner_custom_box() { global $post; $a = new WP_Query('post_type=page'); while($a->have_posts() ) : $a->the_post(); endwhile; wp_reset_postdata(); }
This happens because $wp_query->post is never defined in the admin load, which wp_reset_postdata relies on to reset the original post data.
I am attaching a patch that defines $wp_query->post after the $post global has been defined.
Attachments (6)
Change History (57)
#4
@
13 years ago
Why was this set to invalid? I'm having the same issue.
My solution has been to save the original post in $original_post, then at the bottom of my metabox put setup_postdata( $original_post );
While this works, it would be much better to be able to use wp_reset_query or wp_reset_postdata.
If this ticket is invalid, what's the recommended approach for restoring the query in a metabox?
#5
@
13 years ago
I set it to invalid because the proper thing to do in this situation is to use get_posts() anywhere in the back-end, and not use any template tags.
#6
@
13 years ago
Regardless of what method you use to generate the loop, if you set the post data to something else you'll need to reset it. Using get_posts:
$original_post = $post; $myposts = get_posts( $args ); foreach( $myposts as $post ) : setup_postdata($post); ... endforeach: setup_postdata( $original_post);
Using WP_Query (how I'm doing it):
$original_post = $post; $loop = new WP_Query( $args ); while( $loop->have_posts() ): $loop->the_post(); ... endwhile; setup_postdata( $original_post );
I think it's reasonable to assume that most of the time you're doing a query in a metabox, you'll need access to the post data.
#7
@
13 years ago
Bill - you can access all post data via the $myposts object in the foreach loop without using the setup_postdata() as class variables of each $post, I believe setup_postdata just makes it more simple to access via template tags.
I do agree with you that it would be nice to use template tags in meta boxes, just to keep things cleaner and one less thing to remember.
#8
@
13 years ago
Ah I see, I never use get_posts so was basing that simply on the examples of the codex page.
Nonetheless, I still think it's a valid complaint.
#9
@
11 years ago
- Component changed from Editor to Options, Meta APIs
- Resolution invalid deleted
- Status changed from closed to reopened
- Version changed from 3.2.1 to 3.8.1
This is not invalid. I was able to replicate this in 3.8.1. While using an explicit WP_Query object in a meta box, you cannot reset the postdata after the loop, no matter what you try. Any subsequent meta boxes identify global $post as the last post of the custom query.
@ericlewis assertion that WP_Query and template tags should not be used in wp-admin is not documented anywhere in the Codex that I can find nor in any posts by core contributors. Multiple loops using the an explicit creation of a WP_Query object in wp-admin post screens should work just like any other query method or function.
This is causing issues for other plugins that register meta boxes and generating support tickets like the following: http://wordpress.org/support/topic/plugin-wordpress-seo-by-yoast-after-update-it-wont-save-seo?replies=32
I identified the noted support example here after trying to save post meta for WP SEO but the meta that surfaced after update was based off the post ID of the last post in my custom query in the previous meta box. After a look under the hood, the meta was actually saved properly to the origin post ID as it was a form option in the DOM already.
Using get_posts instead of new WP_Query works, but that's just applying a bandaid fix when this should work out-of-the-box. It's my opinion that this needs to be fixed rather than say, 'use another query method'. Plugin authors will continue to use WP_Query in meta boxes and it will continue to break the main query for the subsequent page load.
#12
@
10 years ago
Is this still an issue? I can't figure out how to reproduce it. When registering a metabox containing a WP_Query
using the code provided by ericlewis, the edit post form on post.php is filling in the data from the expected post. Maybe I'm doing it wrong?
#13
@
10 years ago
In 2011, the right sidebar was output in markup before the main title/content. This has been switched sometime in the last three years (maybe during mp6?), so the bug doesn't express itself so clearly now.
See attachment:18408.php, which clearly shows the_title()
after the wp_reset_postdata()
is using data from the custom WP_Query, not the original post to be edited.
#14
@
10 years ago
- Keywords 2nd-opinion added; dev-feedback removed
Thanks, ericlewis. Now I see what's going on.
The root issue is that wp_reset_postdata()
is designed to reset globals to match the main query. But in the case of post.php, there is no main query - the $post
global is populated manually, using get_post()
, rather than through the normal $wp_query
loop. So perhaps the most straightforward way to resolve this is by doing a "real" $wp_query
loop on edit.php. See 18408-wp-query-in-admin.php. This is similar to your original suggestion to modify get_default_post_to_edit()
, but far more targeted.
The good thing about this solution is that WP_Query
instances on edit.php now work exactly like WP_Query
instances anywhere else. The bad thing is that it's kinda arbitrary. On the front end, the WP bootstrap process (see especially WP::main()
) fires up the main WP query based on the query vars parsed out of the URL. If we fake it on post.php as I've suggested in the patch, we're reproducing only part of this process, which could lead to other bits of confusion down the line. (For example, if you see a $wp_query
global, do you also expect to find a $wp->matched_rule
?)
It's possible that there would be odd side effects of the suggested $wp_query
override - in particular, I guess, if a metabox is manipulating the global in some funny way. That being said, I'm having a hard time thinking of any realistic examples of possible breakage.
What do others think?
#15
@
10 years ago
I just ran into this inconsistency for a plugin I have that's doing some things in the admin bar. Originally it was only pulling a list of pages and adding their edit links to an admin bar item, but I had a feature request for the ability to add other post types to this item. I was doing so with WP_Query
but found that it was breaking the edit screen in that the editable content was being pulled from the last post in my list of items in the admin bar. I thought that wp_reset_postdata()
wasn't functioning properly at first but then realized what was going on.
I agree with @boonebgorges and @misternifty that this does need to be fixed so that the main query behavior is consistent across the admin area. It doesn't make sense for WP_Query
to work on some but not all pages.
#16
@
10 years ago
I experienced the same bug when I tried to change the Toolbar items inside the "wp_before_admin_bar_render" action hook. I wanted to add one custom type posts to the toolbar, and of course I used WP_Query, but that caused this bug to occur in posts edit screen.
I ended up using "get_posts" instead of "WP_Query" and that solved the problem. BUT this bug is not acceptable, and it should be fixed!
#18
in reply to:
↑ 17
@
10 years ago
Replying to boonebgorges:
So FutureRelease - does it mean the fix is in place, waiting for the next release? It's a 4-year-old bug, and we just wasted several hours trying to figure out what's going on before finding this ticket.
#19
@
9 years ago
This behaviour caused me some head scratching as well. Added a note to http://codex.wordpress.org/Class_Reference/WP_Query.
#20
@
9 years ago
- Keywords close added; 2nd-opinion removed
I can't begin to imagine what would break if we did:
global $wp_query; if ( isset( $wp_query ) ) { $wp_query->reset_postdata(); } else { // CODE that is certain to break untold # of plugins and themes }
Globals are super-fragile. I don't think we should start actively supporting theme functions and nested loops with proactive reset on this screen. You're better off toggling the global $post
in your own code here.
#21
@
9 years ago
- Keywords ongoing added; close removed
This could be part of a larger discussion. I also want to immortalize EAL's first bug report.
#22
follow-up:
↓ 23
@
9 years ago
- Keywords needs-patch added; has-patch removed
- Version 3.8.1 deleted
I am just facing to this trouble in version 4.4.1, and wondering how it is going? Had to change the $context in my meta_box to "normal" or keep default setting "advanced".
#23
in reply to:
↑ 22
@
9 years ago
Replying to hungpham:
I am just facing to this trouble in version 4.4.1, and wondering how it is going? Had to change the $context in my meta_box to "normal" or keep default setting "advanced".
My temporary solution is to add the following code in the same function/method that registers the metaboxes:
// the fix
global $post, $wp_query;
$wp_query->post = $post;
// add metaboxes
add_meta_box(...)
add_meta_box(...)
#24
in reply to:
↑ description
;
follow-up:
↓ 25
@
8 years ago
- Version set to 4.7.2
Any idea about this? I've encountered something similar.
I'm trying to populate a metabox dropdown with the titles of a custom post type.
The post which will hold this data is also a custom post type.
It works fine not using wp_query and hard coding the options, but when I use wp_query, there are issues in saving. It populates the dropdown, but the permalink is changed and the dropdown is reset/cannot be saved. It also affects the title sometimes.
#25
in reply to:
↑ 24
;
follow-up:
↓ 26
@
8 years ago
To bypass this issue, do not use the_post
as this is the issue.
I use this syntax and I have numerous metaboxes with no issue to the default post object nor the need to call wp_reset_postdata();
.
<?php $query = new \WP_Query($args); if ($query->have_posts()) { foreach ($query->get_posts() as $p) { // nomal loop logic using $p as a normal WP_Post object } }
It needs to be noted that you cannot use the loop methods like get_the_title()
but you can easily call $p->post_title
. I do not believe the filters are applied to these so you may need to use a different object for the backend than the frontend since the issue is only on the backend of WP.
Also, you could just use the same args with get_posts();
#26
in reply to:
↑ 25
;
follow-up:
↓ 27
@
8 years ago
This works perfectly. The problem persists with 4.7.4 as well.
Replying to jpSimkins:
To bypass this issue, do not use
the_post
as this is the issue.
I use this syntax and I have numerous metaboxes with no issue to the default post object nor the need to call
wp_reset_postdata();
.
<?php $query = new \WP_Query($args); if ($query->have_posts()) { foreach ($query->get_posts() as $p) { // nomal loop logic using $p as a normal WP_Post object } }It needs to be noted that you cannot use the loop methods like
get_the_title()
but you can easily call$p->post_title
. I do not believe the filters are applied to these so you may need to use a different object for the backend than the frontend since the issue is only on the backend of WP.
Also, you could just use the same args with
get_posts();
#27
in reply to:
↑ 26
@
7 years ago
Can confirm that in Wp 4.8.1, this issue is still present. What caused me to notice, was the fact that on save, the currently edited post/page's slug was changing to match the slug of the last post in the Wp_query I was running in the post editor. My meta box was hooked into low priority in the sidebar section of the editor.
What's particularly strange is, if you do something like
<?php get_the_title($p->Id); ?>
Then you get the desired/expected result.
This solution saved my bacon, but the issue itself cost me about an hour of debugging, trying to find the culprit. Issue occurred on my local Apache hosted dev environment with no plugins and only a custom theme I was working on. If it at all helps, I've also noticed issues in other areas of wp's backend when dealing with custom wp_query's, most noticably in other plugins like Advanced custom fields.
Replying to nitesh.luharuka:
This works perfectly. The problem persists with 4.7.4 as well.
Replying to jpSimkins:
To bypass this issue, do not use
the_post
as this is the issue.
I use this syntax and I have numerous metaboxes with no issue to the default post object nor the need to call
wp_reset_postdata();
.
<?php $query = new \WP_Query($args); if ($query->have_posts()) { foreach ($query->get_posts() as $p) { // nomal loop logic using $p as a normal WP_Post object } }It needs to be noted that you cannot use the loop methods like
get_the_title()
but you can easily call$p->post_title
. I do not believe the filters are applied to these so you may need to use a different object for the backend than the frontend since the issue is only on the backend of WP.
Also, you could just use the same args with
get_posts();
#29
@
6 years ago
This issue still exists in WordPress 5.0, with both the Classic Editor and Block Editor.
You can reproduce the issue with this plugin: https://gist.github.com/billerickson/21ff280fb26c3b98120044d53f4e1b61
Classic Editor screenshot: https://cl.ly/e122cffed719
Block Editor screenshot: https://cl.ly/33bd55ca2869
This caused issues in Gutenberg blocks as well (see https://github.com/WordPress/gutenberg/issues/7468 ), and was temporarily fixed using the same approach outlined above - backup the global $post and restore it afterwards (see https://github.com/WordPress/gutenberg/pull/7889 ).
#30
@
5 years ago
This problem is very frustrating, and affects a lot of plugins. For example, Yoast SEO processes shortcodes on restoring a revision, which can result in being redirected to the wrong page after the revision is restored.
And they don't want to fix it, given it's a WordPress core issue: https://github.com/Yoast/wordpress-seo/issues/12017
The approach above was suggesting setting up an entire WP_Query
loop on the admin page, which I can imagine could be tricky.
Rather than having to add hacky code throughout all plugins and a large number of other locations to backup and save the $post variable every time it might change, how about simply doing this once? When $post
is first created in the admin, save it; then adjust the wp_reset_postdata
function to restore it if you're in the admin.
That way plugins/shortcodes can continue to use wp_reset_postdata
in the normal way.
#31
@
4 years ago
In the admin panel the global $wp_query
is technically still available for use. We could simply store the global $post
during the loop_start
hook when specific flags are set ( is_admin()
? ):
add_action( 'loop_start', function() { if( ! is_admin() ) { return; } global $post, $wp_query; $wp_query->post = $post; } );
The above is a simple example but it seems like the global $wp_query
is kept empty in most if not all admin panel requests. This would then allow the use of secondary WP_Querys and wp_reset_postdata()
.
#33
@
4 years ago
Just wanted to chime in with an extra thought here. I've long considered this a core WordPress bug - that wp_reset_postdata
is the correct way for shortcodes, dynamic blocks, or admin code to run their own custom queries.
I've recently discovered, however, that it is not just the WordPress admin that is affected by the call. If completely ordinary theme code runs a custom loop and calls, for example, the_excerpt
(which is empty, and thus processes the full content in order to generate an automatic one, and may run shortcodes etc), the $post
variable is also broken as it is reset to the incorrect value.
Is the true situation here that there is no admin bug at all - and indeed it is *incorrect* for any shortcode, dynamic block, or custom code in the admin, to use wp_reset_postdata
in the first place? (With the solution by eg Bill Erickson - save the $post
variable and restore it afterwards - the correct coding method?)
#34
@
4 years ago
Sorry, ignore the above. Just realised excerpts strip blocks and shortcodes, so it doesn't affect the front-end after all.
#35
@
21 months ago
- Keywords has-patch added; needs-patch removed
This patch allows developers to reset custom WP_Querys back to the original admin post using wp_rest_postdata()
.
This ticket was mentioned in Slack in #core by howdy_mcgee. View the logs.
21 months ago
This ticket was mentioned in Slack in #core by howdy_mcgee. View the logs.
20 months ago
#39
@
20 months ago
- Keywords early added
As this ticket proposes changes to WP_Query
global, a commit should land early
in the dev cycle to allow for a longer amount of time for testing and feedback.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
19 months ago
#41
@
19 months ago
- Owner changed from ericlewis to audrasjb
- Status changed from reopened to assigned
As per today's bug scrub.
The proposed patch still needs testing. Reassign to myself.
#42
@
18 months ago
- Keywords needs-unit-tests added; ongoing removed
This would probably need some unit tests, too.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
18 months ago
#44
@
18 months ago
- Milestone changed from 6.3 to 6.4
As per today's bug scrub, we're gonna move it to milestone 6.4
, pending unit tests and manual testing.
This ticket was mentioned in Slack in #core by oglekler. View the logs.
16 months ago
#46
@
16 months ago
Hi @Howdy_McGee this ticket was reviewed during bug scrub. It needs to be done early due to impact it can possibly have, and also need to be covered with unit test, plus testers need an instruction how to check it. Can you please follow on this? Thank you 🙏
Add props too: @hellofromTonya @zunaid321
#47
@
15 months ago
- Milestone changed from 6.4 to 6.5
Because this is an early ticket, I am moving it into the 6.5 milestone. If there will not be progress, next time it should be rescheduled to the Future release.
This ticket was mentioned in Slack in #core by abhanonstopnews. View the logs.
13 months ago
#49
@
13 months ago
This ticket was discussed during bug scrub.
It still needs unit tests and testing instructions. Because this is an early ticket, it was decided to highlight it in the Dev chat.
Add props to @webcommsat
#50
@
13 months ago
The latest patch moves away from a hook-based approach and adds the association directly to post.php. This should account for the following cases:
- Direct query overrides with the not-recommended query_posts() and wp_reset_query()
- Secondary queries with WP_Query() and wp_reset_postdata()
- Global post overrides with setup_postdata( $post ) and wp_reset_postdata().
I agree with @oglekler that this should be moved to Future release until Unit Tests can be added.
wp-admin/includes/post.php