Typo in class-walker-nav-menu-checklist.php

[yanngarcia]

File: wp-admin\includes\class-walker-nav-menu-checklist.php:114

The name attribute of the "Title Attribute" hidden input ends with ...[menu-item-attr_title] and should ends with ...[menu-item-attr-title] (dash instead of underscore as expected in wp-admin\js\nav-menu.js and wp-admin\includes\nav-menu.php)

Found this typo by wanting to set a default title attribute to a menu item, using the filter 'wp_setup_nav_menu_item', but the input value is not sent/used because of the typo.

[yanngarcia]

Typo fix

[davidbaumwald]

This ticket was discussed in today's scrub. @ipstenu was kind enough to grab a report on impact of plugins referencing the class name here.

Matches  Plugin                      Active installs
=======  ======                      ===============
      1  easy-custom-sidebars                30,000+
      1  userswp                              6,000+
      1  widget-menuizer                        300+
      1  chapters                                60+
      2  defeed                                  10+
      2  widgets-in-menu                     10,000+
      4  mobiloud-mobile-app-plugin           1,000+
      1  buddypress                         200,000+
      1  advance-menu-manager                   700+
      2  menus-multisite-picker                   0+
      1  posterno                                10+
      1  ultimate-member-navigation-menu        200+
      4  wp-widget-in-navigation                600+

@ipstenu also asked about a phased approach for implementation of the fix, given the impact...

[I]s there a way to support both for 5.3 and then drop the typo in 5.4?

[davidbaumwald]

After some digging into this with @desrosj, we found the following:

1. When adding items to a menu, each post in the list has a checkbox. Each post list item also has hidden fields of data related to that post.
2. When the "Add To Menu" button is clicked, The JavaScript method getItemData is run on each hidden field to create an array of data for that post based on these hidden fields.
3. Eventually, api.addItemToMenu is called for each item requested to be added to the menu. This passes the array of data about each menu item to the AJAX request.
4. The AJAX action is add-menu-item which passes the array of data about the item to the Core function wp_save_nav_menu_items.
5. This function is looking for a key of menu-item-attr-title.
6. Data is then passed to another Core function, wp_update_nav_menu_item, to save the menu item post in the DB. This function is also referencing the menu-item-attr-title array key.
7. The post_excerpt is set to whatever is passed.

TL;DR - The value of the hidden field name="menu-item[XXX][menu-item-attr-title]" is intended to be the post excerpt. That value is passed through to the eventual nav_menu_item post when it's saved to a menu. However, it's never going to pass through this value because functions are looking for the key menu-item-attr-title, not menu-item-attr_title. This bug has been unnoticed most liekly because there's not yet been a use-case for using the post excerpt on a menu item.

In looking at the top three plugins that reference the existing key name, all three seem to have propagated the typo in their plugins by simply using the Walker_Nav_Menu_Checklist as a template for a custom walker similar in function.

The fix will need a small call out on the miscellaneous dev note for 5.3.

[davidbaumwald]

There appears to be no reference to the misspelled key anywhere else in the code base, including any CSS.

Given that that the original patch is still applying cleanly, this is being reassigned to @desrosj at his request for merging. This will still need the small call out to alert plugin devs of the update.

[desrosj]

Thanks, @davidbaumwald! Went back and it looks like this was introduced in [14248], so has been broken for some time.

[desrosj]

In 46380:

Menus: Fix typo in the class attribute for the hidden title field in Walker_Nav_Menu_Checklist.

Each item that Walker_Nav_Menu_Checklist displays is accompanied by several hidden <input/> fields that specify default values for each item when added to a menu. These values are passed in JavaScript to the AJAX call triggered when an item is added to a menu.

The hidden field for the title attribute field incorrectly had an underscore instead of a hyphen. Because of this, it was impossible to supply a default value for the Title Attribute field of a nav menu item.

Props yanngarcia, davidbaumwald.
See #47838.

[desrosj]

Accidentally did not include Fixes in [46380]. Closing out, dev note to follow.

[SergeyBiryukov]

In 46401:

Customize: Remove duplicate attr_title assignment in api.Menus.MenuItemControl.renderContent().

See #47838.

[desrosj]

Mentioned in the Miscellaneous Developer Focused Changes dev note for 5.3: ​https://make.wordpress.org/core/2019/10/15/miscellaneous-developer-focused-changes-in-5-3/