[wp-trac] [WordPress Trac] #43113: Multiple custom item classes are returned as single string when using 'nav_menu_link_attributes' filter with Customizer preview

[WordPress Trac]

#43113: Multiple custom item classes are returned as single string when using
'nav_menu_link_attributes' filter with Customizer preview
-------------------------------------------------+-------------------------
 Reporter:  lrdn                                 |       Owner:  noisysocks
     Type:  defect (bug)                         |      Status:  reviewing
 Priority:  normal                               |   Milestone:  Future
                                                 |  Release
Component:  Customize                            |     Version:  4.3
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests needs-      |     Focuses:
  testing needs-refresh                          |
-------------------------------------------------+-------------------------
Changes (by noisysocks):

 * keywords:  has-patch has-unit-tests needs-testing => has-patch has-unit-
     tests needs-testing needs-refresh
 * owner:  (none) => noisysocks
 * status:  new => reviewing


Comment:

 Replying to [comment:3 dlh]:
 > Storing the classes as a string in the setting value seems right to me
 because `wp_update_nav_menu_item()` expects to receive a string for `menu-
 item-classes`.

 Agreed.

 > The problem occurs (I think) when the nav menu item's properties are
 overwritten with the customized state in
 `\WP_Customize_Nav_Menu_Item_Setting::filter_wp_get_nav_menu_items()`. The
 string setting value for `classes` overwrites the array value set by
 `wp_setup_nav_menu_item()`.

 Yes, that seems right.

 `wp_update_nav_menu_item()` is ordinarily what will turn the `classes`
 string on the menu item object into an array.

 During preview, however, the menu item object is set up by
 `WP_Customize_Nav_Menu_Item_Setting::preview()` →
 `WP_Customize_Nav_Menu_Item_Setting::filter_wp_get_nav_menu_items()` which
 bypasses `wp_update_nav_menu_item()`. Assumedly this is so that no writes
 to the database are made.

 > [attachment:"43113.diff"] attempts to fix this in
 `\WP_Customize_Nav_Menu_Item_Setting::value_as_wp_post_nav_menu_item()` by
 ensuring that a string `classes` is exploded into an array before it's
 used to overwrite the property.

 [attachment:"43113.diff"] no longer applies cleanly. Also, I think we
 should use `! is_array()` instead of `is_string()` on the very off chance
 that `$classes` is a number or something silly.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/43113#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform