Make WordPress Core

Opened 3 years ago

Last modified 3 years ago

#55051 new defect (bug)

WP_Customize_Nav_Menu_Item_Setting class needs hooks/filters parity for preview and update

Reported by: helgatheviking's profile helgatheviking Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.3
Component: Customize Keywords: needs-patch
Focuses: Cc:

Description

the WP_Customize_Nav_Menu_Item_Setting class needs parity with the WP_Customize_Setting class.

The following actions exist in the WP_Customize_Setting class:

do_action( "customize_preview_{$this->id}", $this );
do_action( "customize_preview_{$this->type}", $this );

[see source](https://github.com/WordPress/WordPress/blob/234877c9c3d81b6341bef5539ef52b0745b2a660/wp-includes/class-wp-customize-setting.php#L389-L411)

and
do_action( "customize_update_{$this->type}", $value, $this );

[see source](https://github.com/WordPress/WordPress/blob/234877c9c3d81b6341bef5539ef52b0745b2a660/wp-includes/class-wp-customize-setting.php#L692-L703)

The preview() and update() methods are overriden in WP_Customize_Nav_Menu_Item_Setting and don't have those do_action hooks, so there's no straightforward way to filter nav menu meta values on preview/update.

For example, the Nav Menu Roles plugin adds meta fields to the Nav Menu Items in the customizer and am currently using a workaround to attach the preview/save routines since there's no direct way to do this with these hooks missing.

Potentially related, the WP_Customize_Nav_Menu_Item_Setting class has an unimplemented @todo in the preview() method: see [source](https://github.com/WordPress/WordPress/blob/071c322bf2211db37cb38b4ddf4d2ed660e745d6/wp-includes/customize/class-wp-customize-nav-menu-item-setting.php#L461)

I'm not 100% certain what is meant there and if it can be ignored if those hooks are implemented. (a quick test suggests that I can filter the metadata in a callback on customize_preview_nav_menu_item once it exists.

Change History (4)

#1 @dlh
3 years ago

  • Component changed from General to Customize
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.0

Thanks for the report! I agree that it's inconsistent that the hooks don't fire in the overridden methods. I also, though, observe that the same inconsistency seems to exist in all of WP_Customize_Setting's child classes, e.g. WP_Customize_Custom_CSS_Setting::update(), so perhaps there was some purpose behind it. Worth investigating!

#2 @helgatheviking
3 years ago

I spent all day trying to make sense of it yesterday and while I didn't make much sense of anything :) I noticed the same thing... the hooks are inconsistent across the extended classes.

I also was trying to figure out how the input data flows and I think it goes from sanitize to update... but I find the sanitize filter pretty difficult to work with. How would one know what each nav menu item's ID is going to be in order to filter ["customize_sanitize_{$this->id}"](https://github.com/helgatheviking/Nav-Menu-Roles/tree/issues/56-exclude-by-role)? And how does one get the current data there as $menu_item_value is the sanitized result.

#3 @costdev
3 years ago

  • Version set to 4.3

#4 @peterwilsoncc
3 years ago

  • Milestone changed from 6.0 to Future Release

As RC1 is approaching next week, I am moving this to a future release as it still requires a patch.

Note: See TracTickets for help on using tickets.