Customizer: Create New Menu is Empty

[boldgrid]

When attempting to create new menus in the WordPress Customizer, the Create New Menu panel is empty. I am not getting any console errors or php errors.
Steps to Replicate:

1. Install clean wordpress with latest stable version.
2. Install beta tester plugin.
3. Set to bleeding edge nightlies
4. Dashboard > Updates > update core ( this installed build 5.6-beta1-49262 for me )
5. Go to Appearence > Customize
6. Go to Menus > Create New Menu

[boldgrid]

Gif screencapture demonstration issue.

[dlh]

Welcome to Trac, @boldgrid!

According to git bisect, the trouble here started with [49101].

[azaozz]

Yeah, can reproduce it too. It is caused by using jQuery 3.5.1 plus jQuery Migrate 3.3.1. I see that some of the HTML there is missing but can't see where that comes from. Will keep digging.

[boldgrid]

Making the following additions to wp-admin/js/customize-nav-menus.js resolved the issue for me.

L1361+: menuNameControl.renderContent();
L1375+: menuLocationsControl.renderContent();
L1388+: newMenuSubmitControl.renderContent();

[Clorith]

Spent some time digging, but couldn't find the root cause either yet, but some more details that have been found so far:

The problem appears to be that the wp.template function never builds the menu selection markup, which is needed to make the markup which the jQuery selector later depends on when clicking to change ot the create menu screen.

r41768 introduced a new menu creation flow, reverting back to just before this point, and feeding in jQuery 3.x and things still work as expected, so I wonder if it's something relating to this flow-change which isn't playing nicely with other related JS changes.

[david.binda]

I have looked into this a bit, and it looks like the rendering of the missing nav menu controls is prevented by the conditional in ​https://build.trac.wordpress.org/browser/trunk/wp-admin/js/customize-controls.js?rev=48413#L3701

Testing the beta with jQuery 3.5.1, the parentContainer and control.container.parent() are the same objects, however with jQuery 1.12.4 the control.container.parent() is empty.

Could this be related to the changes mentioned in ​https://jquery.com/upgrade-guide/1.9/#ordering-of-disconnected-nodes-within-a-jquery-set ?

However, I'm not sure if that's a root cause or just a symptom.

[Clorith]

That shouldn't be caused by the 1.9 changes (since it works in 1.12.4-wp).

But the section you found does bubble upwards to a Deferred element, which did have some ​changes in jQuery 3.0, the code uses .done() though, which (if I read this right) should retain backwards compatibility and not have changed behaviors.

[david.binda]

That shouldn't be caused by the 1.9 changes (since it works in 1.12.4-wp).

Right. I was too fast with my judgement there while looking for possible cause upstream. However, if my testing is correct, the code's behaviour changes in between jQuery 2.2.4 and 3.0.0 (as in the .parent() is not returning the parent for control.container ).

The biggest part of the diff in between the 2.2.4 and 3.0.0 is the Deferred element you are mentioning.

However, even in 2.2.4, the parentNode seems to be set (just .parent() is not returning it).

I wonder, can't the control.renderContent(); call be moved outside of the conditional leaving only the .append inside?

In r41482 there seems to be a similar change, however a part of a bigger changeset, so I'm not really sure why it was needed.

[david.binda]

My current understanding is that the control rendering is really broken due to the changes in Deferred objects, as it has already been mentioned.

While we are using .done() in customizer code, we are also using $.where() method (see ​https://build.trac.wordpress.org/browser/trunk/wp-includes/js/customize-base.js?rev=48412#L512 ), which was also updated:

Breaking change and Feature: jQuery.when() arguments
jQuery.when now interprets any input argument with a then method as a Promise-compatible "thenable".

which, IMHO, with changes to .then(),

Another behavior change required for Promises/A+ compliance is that Deferred .then() callbacks are always called asynchronously. Previously, if a .then() callback was added to a Deferred that was already resolved or rejected, the callback would run immediately and synchronously.

(source: ​https://jquery.com/upgrade-guide/3.0/#breaking-change-and-feature-jquery-deferred-is-now-promises-a-compatible )

means that the timing of when our embed method is being called and when the element is appended to the DOM might be slightly different.

That's why I believe that the control.renderContent(); should be called outside the conditional (see the patch).

[azaozz]

Replying to david.binda:
Ha, was just about to attach pretty much the same patch, but you beat me to it! :)

The biggest part of the diff in between the 2.2.4 and 3.0.0 is the Deferred element you are mentioning.

However, even in 2.2.4, the parentNode seems to be set (just .parent() is not returning it).

My "current guess" is that this is timing related. Looking at 5.5, when the deferred in inject runs, none of the control.container[0].parentNode exist yet, or are in a DocumentFragment (not in the DOM yet, so jQuery cannot find the parent). So control.container.parent().is( parentContainer ) is always false. Logs:

sectionId ===  nav_menu[5]
control.container[0].parentNode null
control.container.parent()[0] Object { length: 0, prevObject: {…}, context: undefined }
control.container.parent().is( parentContainer ) false
---------------------------------------- 
sectionId ===  add_menu
control.container[0].parentNode null
control.container.parent()[0] Object { length: 0, prevObject: {…}, context: undefined }
control.container.parent().is( parentContainer ) false
---------------------------------------- 
sectionId ===  add_menu
control.container[0].parentNode null
control.container.parent()[0] Object { length: 0, prevObject: {…}, context: undefined }
control.container.parent().is( parentContainer ) false
---------------------------------------- 
sectionId ===  menu_locations
control.container[0].parentNode DocumentFragment [ li#customize-control-nav_menu_locations-top.customize-control.customize-control-nav_menu_location ]
control.container.parent()[0] Object { length: 0, prevObject: {…}, context: undefined }
control.container.parent().is( parentContainer ) false
----------------------------------------

However in 5.6-beta the same debug logging outputs:

sectionId ===  menu_locations
control.container[0].parentNode DocumentFragment [ li#customize-control-nav_menu_locations-social.customize-control.customize-control-nav_menu_location ]
control.container.parent()[0] Object { length: 0, prevObject: {…} }
control.container.parent().is( parentContainer ) false
----------------------------------------
sectionId ===  nav_menu[10]
control.container[0].parentNode <ul id="sub-accordion-section-nav_menu[10]" class="customize-pane-child acc...">
control.container.parent()[0] Object { 0: ul#sub-accordion-section-nav_menu[10].customize-pane-child.acc..., length: 1, prevObject: {…} }
control.container.parent().is( parentContainer ) true
---------------------------------------- 
sectionId ===  add_menu
control.container[0].parentNode <ul id="sub-accordion-section-add_menu" class="customize-pane-child acc...">
control.container.parent()[0] Object { 0: ul#sub-accordion-section-add_menu.customize-pane-child.acc..., length: 1, prevObject: {…} }
control.container.parent().is( parentContainer ) true
---------------------------------------- 

The nav_menu and add_menu containers are already in the DOM, the other containers are not and control.container.parent().is( parentContainer ) is still false for them.

I wonder, can't the control.renderContent(); call be moved outside of the conditional leaving only the .append inside?

Yes, testing this here seems to work. Pinging @westonruter for a review.

[westonruter]

Replying to azaozz:

The nav_menu and add_menu containers are already in the DOM, the other containers are not and control.container.parent().is( parentContainer ) is still false for them.

I wonder, can't the control.renderContent(); call be moved outside of the conditional leaving only the .append inside?

Yes, testing this here seems to work. Pinging @westonruter for a review.

This code introduced in r30102, with the work having been done in ​https://github.com/xwp/wordpress-develop/pull/29. The message for the ​Git commit was:

Allow controls and sections to be registered without parents

• Add ready() abstract to Section/Panel
• Remove needless callbacks

That being said, I don't recall why control.renderContent() was being made conditional upon parentContainer not being the control.container.parent(). Moving it outside of that conditional makes sense to me, and I'm not seeing any other regressions caused by doing so.

[hellofromTonya]

Notes from scrub today:

Sergey:

There's a patch that I think makes sense to try in Beta 2

@helen

Yeah I definitely don’t want this to continue to be broken in beta
I don’t understand the fix but I will trust other people to tell me it’s fine if they don’t commit it first

[SergeyBiryukov]

Replying to westonruter:

That being said, I don't recall why control.renderContent() was being made conditional upon parentContainer not being the control.container.parent(). Moving it outside of that conditional makes sense to me, and I'm not seeing any other regressions caused by doing so.

It's worth noting that there is a similar fragment for panels​ earlier in the file, where panel.renderContent() is called outside of the conditionals:

if ( ! panel.headContainer.parent().is( parentContainer ) ) {
	parentContainer.append( panel.headContainer );
}
if ( ! panel.contentContainer.parent().is( panel.headContainer ) ) {
	container.append( panel.contentContainer );
}
panel.renderContent();

So the patch brings some consistency and works as expected in my testing. Marking for commit.

[SergeyBiryukov]

In 49323:

Customize: Ensure the New Menu section is properly rendered.

Follow-up to [30102], [49101].

Props david.binda, boldgrid, dlh, azaozz, Clorith, westonruter, hellofromTonya.
Fixes #51592.