Use of the_post() in _wp_ajax_menu_quick_search() is making in_the_loop = true

[ruud@…]

In /wp-admin/includes/nav-menu.php the menu quick search results are transfered to a json response by doing a search and looping over the results with a 'regular' have_posts(): the_post() construction.
As a side-effect the in_the_loop gets set to true in the_post() function.

This being an admin facing display of (custom)posts titles, I didn't suspect this behavior.

If this is indeed not meant to be, then I can think of 2 solutions:
1) Quick (and dirty?) add an if ( is_admin() ) check to the_post(), to prevent accidental setting of 'in_the_loop' while in the admin.
2) Don't use the_post() in _wp_admin_menu_quick_search().

For the latter I did some looking around in the admin for other similar situations and cooked up the attached patch.

While in this function I did some further code tidying.

[ruud@…]

nav-menu.php patch

[jeremyfelt]

It is interesting that we use the loop like that in the quick search. Is there a specific scenario where this becomes a problem?

Related, this was introduced in #12713

[ruud@…]

The scenario in which this became apparent (and a problem) is as follows:
We use a naming convention where a page/post title can have additional info enclosed with square brackets, e.g.: "Reception [senior department]"

We have multiple 'Reception' pages throughout our website for each department. This helps us greatly in some cases where page/post titles are shown in CMS dropdowns etc. the bracket designation helps identify the page you want (instead of random choosing 1 of perhaps 6 'Reception' pages)

To prevent showing this title to an end-user we use the 'the_title' filter with a 'in_the_loop' check. The filter then removes the square bracket part of the title.

[ruud@…]

Patch refreshed to current trunk version (37802)

[ruud@…]

refresh for patch

[ruud@…]

Just had a quick discussion at WCEU contributor day 2016 with @jeremyfelt. The patch may be fixing this, however the usage of query_posts itself is also a bit odd. So instead of fixing this badly I'm going to propose another patch which removes the usage of the query_posts entirely

[ruud@…]

patch using WP_QUERY instead of query_posts()

[ruud@…]

Using WP_Query instead of query_posts seems like a more performant way of obtaining this small list of pages. Also this solves the initial 'problem' of having a 'in_the_loop=true' value.
Somehow the 'in_the_loop=true' feels like a side-effect of the former/old method of getting posts for this specific purpose.

[ocean90]

In 37881:

Nav Menus: Use WP_Query for quick searches.

the_post() sets the $in_the_loop property to true which is unexpected in the admin if you're using filters which should only affect real loops.

Props ruud@joyo.
Fixes #27042.