in WP 3.5.1, filter wp_nav_menu_items is never called on empty menus

[mykle]

We upgraded from 3.4 to 3.5.1 and our main navigation menu vanished.

Our theme uses the "wp_nav_menu_items" filter to add menu items to an empty menu, depending on login context & other things.

I have verified that in 3.5.1, this filter is never called on empty menus. wp_nav_menu() called with the proper args to select our menu just returns null. we are specifying theme_location in the call.

A workaround/hack is to add a bogus menu item to the DB, via the admin interface, and then delete that bogus menu item in our filter.

I imagine this crept in with the new 3.5 behavior of not emitting HTML for an empty menu.

The bug is in wp-includes/nav-menu-template.php . The comment at line 174 claims we will fallback on empty menus only if no theme_location is specified, and we do test that on line 177, but on line 181 we test again and fallback on empty menus without checking theme_location.

This line (181) :

if ( !$menu || is_wp_error( $menu ) || empty( $menu_items ) )

... should be this line:

if ( !$menu || is_wp_error( $menu ))

[SergeyBiryukov]

Introduced in [21868].

[DrewAPicture]

This is intended behavior. If the menu is empty, don't display the markup.

The comment at line 174 claims we will fallback on empty menus only if no theme_location is specified, and we do test that on line 177, but on line 181 we test again and fallback on empty menus without checking theme_location.

We 'test again' after the fallback callback is fired, which I'd argue is what you should be using to bring menu items in to an empty menu. As noted in #21576, outputting empty markup for menus can wreak havoc on theme styling that relies on wp_nav_menu falling back to the callback.

Suggest close.

[mykle]

When the theme location is specified, the fallback is not fired.

I would argue (politely!) that the menu filters are for changing the contents of a menu, and that adding contents to an empty menu is just as legitimate a use of those filters as any other. If that's true, then you can't really say a menu is empty until you've called its filters. That has been normal behavior up to 3.5 .

Furthermore: the doc specifies the purpose of the callback is for "if a menu doesn't exist". In this case the menu definitely exists, and simply has no contents until after the filters fire.

If the menu is still empty after its filters run, it should certainly not be output. But otherwise, this change silently deactivates a long-standing WP feature.

But maybe we're doing it wrong. What's a better approach to populating an empty menu via a plugin?

[DrewAPicture]

Replying to mykle:

I would argue (politely!) that the menu filters are for changing the contents of a menu, and that adding contents to an empty menu is just as legitimate a use of those filters as any other. If that's true, then you can't really say a menu is empty until you've called its filters. That has been normal behavior up to 3.5 .

I think that's a valid argument. On the other hand, reverting the change from [21868] doesn't maintain both what you're aiming for and hiding the markup for an empty menu.

I can see moving the filter earlier or bailing later in the function, but there may be wider back-compat implications to consider. Let's see what @nacin or @SergeyBiryukov have to say.

[nacin]

We should aim to fire any pertinent filters prior to bailing. The goal of [21868] was to avoid markup, not improve performance.

[DrewAPicture]

24035.diff​ moves the return after the menu item filters.

[DrewAPicture]

Bail after menu items filters have run (against 3.5.2 tag)

[nacin]

(against 3.5.2 tag)

In general, always make patches against trunk. Patches against a branch can be in addition to, but not in lieu of.

[nacin]

In 24634:

If a nav menu has no items, wait until after the wp_nav_menu_items filter before deciding whether to print nothing.

see [21868] for original commit. see #21576.
see #24035 for trunk.

[andg]

The bug appears to be already fixed in trunk.