Empty `items_wrap` `wp_nav_menu` randomly breaks customizer

[ojrask]

Assuming a navigation menu is generated in the frontend using wp_nav_menu and setting the argument items_wrap to an empty string ('').

Then when toying around with the menu and its items inside the customizer, the customizer breaks randomly and begins duplicating the whole menu recursively in the preview pane until the browser crashes or is forced to quit.

If the items_wrap argument is removed or changed the problem disappears.

Tested on WP 4.4 with a custom theme.

[westonruter]

@ojrask: Thanks for the report.

I don't see how setting items_wrap to an empty string would work at all in the first place. When I try, the menu stops appearing altogether.

The default value for items_wrap I see is <ul id="%1$s" class="%2$s">%3$s</ul>. This then is used in wp_nav_menu() via:

<?php
$nav_menu .= sprintf( $args->items_wrap, esc_attr( $wrap_id ), esc_attr( $wrap_class ), $items );

So if the items_wrap is empty, then no menu will be constructed at all from the format string.

I am able to remote the wrapper element by setting items_wrap to '%3$s'.

Are you removing the container in addition to the items_wrap? You could then actually be noticing what has been described here: ​https://make.wordpress.org/core/2015/07/29/fast-previewing-changes-to-menus-in-the-customizer/#comment-26503

[ojrask]

Hey,

So sorry, I misunderstood the issue details from a team mate. Yes, the container is set to false and items_wrap contains only %3$s.

The menu began working normally after the default items_wrap was used and container was still false. Other menu args are fallback_cb => false and theme_location => foobar.

It seems that the issue is the with the "fast refresh" system as you mentioned.

[westonruter]

I believe the primary fix here is to just block partial refresh for nav menus that have container and items_wrap args that both lack a start tag, as in 35362.0.diff​. Granted this will then still fail if the one of the elements does contain a start tag but then lacks having a class attribute. I'm working on a more robust solution in #27355.

[westonruter]

@ojrask the Customize Partial Refresh plugin should now include the fix for this. Can you test for the issue with the plugin active and on the latest nightly (4.5-alpha)?

[westonruter]

There is a different problem now with the merge of selective refresh. No longer will the menu get duplicated. Now the problem is that when a wp_nav_menu() is called in a way that it cannot be selective-refreshed, then it will fail to get a full page refresh. This means that changes to such menus currently cannot be previewed in 4.5-beta2.

Some of the logic removed in [36586] may need to be restored, specifically the logic which kept a tally of the total number of wp_nav_menu() calls not just those which can use selective refresh.

[westonruter]

@ojrask are you able to do some testing on this latest patch to ensure that it works with your configuration?

[westonruter]

Discovered an issue: In Twenty Fifteen with the Social Links Menu assigned to no menu, assign a nav menu to that location and notice the preview not update to show the nav menu.

[westonruter]

In 35362.3.diff​:

• Do fallback behavior if no menu is associated with the nav menu instance partial.
• Do fallback behavior if newly-rendered menu is empty
• Do full refresh if a nav_menu_locations[] is changed for a location which doesn't exist in the preview

[westonruter]

Another edge case discovered:

1. Create a Custom Menu widget and assign a menu to this widget that lacks any menu items.
2. Add a menu item to this widget and notice that the menu is not updated in the widget.

[westonruter]

​https://github.com/xwp/wordpress-develop/commit/cf425089830e5938ea8c629b6d5dcf6effe47ede

[westonruter]

35362.4.diff​: Do partial fallback when adding nav menu items to empty menus

Another edge case:

1. Create nav menu widget and assign to a non-empty nav menu
2. Change the widget's assigned nav menu to one that lacks any menu items.
3. Add nav menu item to the empty nav menu
4. No changes are seen.

[westonruter]

​https://github.com/xwp/wordpress-develop/commit/75c8013ba9e5660d6c8bd0fb2908987ae78286f5

[westonruter]

35362.5.diff​ Fix aforementioned edge case by ensuring that wp_nav_menu() calls made inside of partials get communicated back and made part of the manifest of instances on the page so that the full-page refresh fallback logic will know about them.

[westonruter]

Tweak comment

[westonruter]

In 36889:

Customize: Fix regressions and harden implementation of selective refresh for nav menus.

• Request full refresh if there are nav menu instances that lack partials for a changed setting.
• Restore WP_Customize_Nav_Menus::$preview_nav_menu_instance_args and WP_Customize_Nav_Menus::export_preview_data() from 4.3, and keeping a tally of all wp_nav_menu() calls regardless of whether they can use selective refresh.
• Ensure that all instances of wp_nav_menu() are tallied, regardless of whether they are made during the initial preview call or during subsequent partial renderings. Export nav_menu_instance_args with each partial rendering response just as they are returned when rendering the preview as a whole.
• Fix issues with Custom Menu widget where nav menu items would fail to render when switching between menus when a menu lacked items to begin with.
• Make sure the fallback behavior is invoked when the partial is no longer associated with a menu.
• Do fallback behavior to refresh preview when all menu items are removed from a menu.

Follows [36586].
See #27355.
Fixes #35362.