WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 2 weeks ago

#18408 reopened defect (bug)

Can't wp_reset_postdata after custom WP_Query in an admin edit page

Reported by: ericlewis Owned by: ericlewis
Milestone: Future Release Priority: normal
Severity: normal Version: 3.8.1
Component: Query Keywords: has-patch needs-testing 2nd-opinion
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 (4)

post.php (60.2 KB) - added by ericlewis 4 years ago.
wp-admin/includes/post.php
18408.patch (1.3 KB) - added by ericlewis 4 years ago.
.patch file version
18408.php (440 bytes) - added by ericlewis 4 months ago.
18408-wp-query-in-admin.php (585 bytes) - added by boonebgorges 4 months ago.

Download all attachments as: .zip

Change History (22)

@ericlewis4 years ago

wp-admin/includes/post.php

@ericlewis4 years ago

.patch file version

comment:1 @ericlewis4 years ago

  • Keywords needs-testing dev-feedback added

comment:2 @ericlewis4 years ago

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

comment:3 @SergeyBiryukov4 years ago

  • Milestone Awaiting Review deleted

comment:4 @billerickson3 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?

comment:5 @ericlewis3 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.

comment:6 @billerickson3 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.

comment:7 @ericlewis3 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.

comment:8 @billerickson3 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.

comment:9 @misternifty13 months 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.

comment:10 @nacin13 months ago

  • Milestone set to Awaiting Review

comment:11 @boonebgorges7 months ago

  • Component changed from Options, Meta APIs to Query

comment:12 @boonebgorges4 months 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?

@ericlewis4 months ago

comment:13 @ericlewis4 months 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.

comment:14 @boonebgorges4 months 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?

comment:15 @brettshumaker2 months 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.

comment:16 @dezio19006 weeks 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!

comment:17 follow-up: @boonebgorges6 weeks ago

  • Milestone changed from Awaiting Review to Future Release

comment:18 in reply to: ↑ 17 @archon8102 weeks 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.

Note: See TracTickets for help on using tickets.