Make WordPress Core

Opened 13 years ago

Closed 12 years ago

#16798 closed defect (bug) (fixed)

Admin bar edit action is affected by query_posts, tax_query

Reported by: kanuck54's profile kanuck54 Owned by: ryan's profile ryan
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.1
Component: Administration Keywords: has-patch
Focuses: Cc:

Description

I don't know if this part is a bug, but I'm guessing yes:

If I run query_posts() in a custom post type's template, I lose the admin bar's "edit" action.

This part, however, is definitely a bug:

If I include a tax_query in my query_posts(), my edit link actually changes to an edit link for the last item in the array that was given to the tax_query, with the label "Edit Category."

Here's the minimum offending code to reproduce, placed into a custom post type template:

query_posts( array(
	'post_type' => 'fp_toy',
	'tax_query' => array(
		array(
			'taxonomy' => 'fp_gender',
			'field' => 'ID',
			'terms' => array(14)
		)
	),
) );

Now my "Edit Toy" button says "Edit Category" instead, and points to edit-tags.php?action=edit&taxonomy=fp_gender&tag_ID=14.

Ideally I would expect the admin bar's edit action to be unaffected by what happens inside the template. And I definitely don't expect a completely unrelated action to show up instead!

Attachments (2)

16798.patch (420 bytes) - added by SergeyBiryukov 12 years ago.
16798.2.patch (612 bytes) - added by SergeyBiryukov 12 years ago.

Download all attachments as: .zip

Change History (12)

#1 @ryan
13 years ago

That all sounds like expected behavior. "Edit Category" shows because a taxonomy was queried, not a particular post. Linking to "/wp-admin/edit.php?taxonomy=category&tag_ID=14" would likely be more useful than linking to edit-tags.php, but either way showing Edit Category is correct here.

If you don't want the admin bar and other areas to reflect queries made within your template, use get_posts() instead of query_posts().

#2 follow-ups: @kanuck54
13 years ago

If that's supposed to be the idea there, I would expect the "edit" link to disappear entirely. It doesn't make a lot of sense to offer me the opportunity to edit whichever of the many categories happens to be last in the list; it's confusing.

#3 in reply to: ↑ 2 @ryan
13 years ago

Replying to kanuck54:

If that's supposed to be the idea there, I would expect the "edit" link to disappear entirely. It doesn't make a lot of sense to offer me the opportunity to edit whichever of the many categories happens to be last in the list; it's confusing.

No argument there. It is not very useful or predictable.

#4 @TJNowell
12 years ago

You should use an instance of the WP_Query() object to make the query, then follow it with a call to wp_reset_query();

http://codex.wordpress.org/Function_Reference/wp_reset_query

That 'should' fix your issue.

Last edited 12 years ago by TJNowell (previous) (diff)

#6 in reply to: ↑ 2 @SergeyBiryukov
12 years ago

  • Keywords has-patch added

Replying to kanuck54:

If that's supposed to be the idea there, I would expect the "edit" link to disappear entirely. It doesn't make a lot of sense to offer me the opportunity to edit whichever of the many categories happens to be last in the list; it's confusing.

wp_reset_query() indeed should be used, but here's a patch to hide the "Edit" link if a subsequent query has been made.

#7 follow-up: @nacin
12 years ago

If you use new WP_Query(), use wp_reset_postdata() to restore the globals.

If you use query_posts(), use wp_reset_query() to restore the query and the globals.

I think the patch is on the right track, but I'm not sure it's catching the right situation. Would it make more sense to use $wp_the_query-> get_queried_object(), perhaps? Or rather, if ( ! is_main_query() ) wp_reset_query();

query_posts() is used all the time, and not particularly often does the page change so much that the Edit link is no longer out of place. (I would guess, anyway.)

#8 in reply to: ↑ 7 @SergeyBiryukov
12 years ago

  • Milestone changed from Awaiting Review to 3.3

Replying to nacin:

Would it make more sense to use $wp_the_query-> get_queried_object(), perhaps? Or rather, if ( ! is_main_query() ) wp_reset_query();

The first option seems less obtrusive. Done in 16798.2.patch.

#9 @kanuck54
12 years ago

Sounds great. I now understand to use WP_Query and its the_post() method, and then restore with wp_reset_postdata(), but I still agree that modifying the query shouldn't affect the admin bar—so thanks for addressing it!

#10 @ryan
12 years ago

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

In [18844]:

Consult the main query when determining whether to show the edit menu in the admin bar. This insulates the admin bar from query_posts() queries made by themes and plugins. Props SergeyBiryukov. fixes #16798

Note: See TracTickets for help on using tickets.