wp_update_nav_menu hook is not fired when nav menu item is auto-added

[inderpreet99]

WP calls the action hook 'wp_update_nav_menu' when a navigation menu is updated. For example, go to Site Design -> Custom Menus -> Click the "Save Menu" button on a menu.

Next to the Menu title, the Custom Menus provide the user to "Automatically add new top-level pages". When this option is checked, and a new top-level page is created, the 'wp_update_nav_menu' hook is not fired. So the attached nav-menu-hooks.diff patch fixes that.

I would also suggest that we do not use the same hook 'wp_update_nav_menu' twice (as it is currently being done) with different amount of arguments, because this causes a PHP Warning

PHP Warning:  Missing argument 2 for x_save_footer() in /var/www/branches/x.trunk/wp-content/themes/x/functions.php on line 815, referer: http://x.com/x/wp-admin/nav-menus.php

for code:

add_action('wp_update_nav_menu', 'x_save_footer', 10, 2);

So, the hook call at nav-menu.php:255 has 2 parameters and the call at nav-menus.php:383 has 1 parameter.

As a fix, the action hook in nav-menu.php should be named something else, like 'wp_update_nav_menu_object' (because it is updating the object), as done in the attached nav-menu-hooks.diff patch. This will also prevent the same hook from being fired twice when updating the nav menus.

[inderpreet99]

nav-menu.php: new hook and hook name change

[SergeyBiryukov]

Renaming an existing hook isn't a good idea, some plugins are using it. I've found two:

wp-super-cache/trunk/wp-cache-phase2.php
add-descendants-as-submenu-items/trunk/add-descendants-as-submenu-items.php

[mark8barnes]

Replying to SergeyBiryukov:

Renaming an existing hook isn't a good idea, some plugins are using it. I've found two:

wp-super-cache/trunk/wp-cache-phase2.php
add-descendants-as-submenu-items/trunk/add-descendants-as-submenu-items.php

But surely having these two actions named the same is a bug, not a design decision. The two hooks fire at completely different times in the save menu process - the first fires when the menu itself is updated, the second fires when the menu items are updated by javascript.

Incidentally, trying to monitor a menu by adding a hook to discover a menu change is way harder than it ought to be - partly for the reason above, and partly because when javascript is disabled, no hooks are in place.

[inderpreet99]

Like @mark8barnes, we should rename the hooks since they're being called at completely different times. This is a bug. It's a pretty important hook, so I think it should be fixed.

Although I don't know why @mark8barnes suggests that javascript is firing one of the hooks. I've disabled JS and I do not see that happening.

If we're not comfortable with renaming the hooks (due to those two plugins), we could just add two new hooks with unique names. This would allow the developers to target a specific hook.

PS: Since it's been 4 years, the hooks have moved around. They are now located in wp-admin/includes/nav-menu.php:1048 and wp-includes/nav-menu.php:346.