Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#49500 closed defect (bug) (worksforme)

WordPress 5.4 messes up custom fields added to menu items they are duplicated.

Reported by: backups's profile BackuPs Owned by: audrasjb's profile audrasjb
Milestone: Priority: normal
Severity: normal Version: 5.4
Component: Menus Keywords:
Focuses: Cc:

Description

Hi

I have a problem with wp 5.4 all the custom fields added to menu items are now suddenly duplicated.

See images

wp 5.3.2 https://cl.ly/e13ad91e0de9

wp 5.4 https://cl.ly/85b8b73ea85a

wp 5.3.2 https://cl.ly/e13ad91e0de9

Wp 5.4 https://cl.ly/85b8b73ea85a

how to fix this?

Change History (21)

#1 @audrasjb
4 years ago

  • Keywords reporter-feedback added; needs-patch removed
  • Owner set to audrasjb
  • Status changed from new to reviewing

Hello,

Could you please provide the code used to add the related custom fields?

#2 @SergeyBiryukov
4 years ago

  • Component changed from General to Menus

#3 @BackuPs
4 years ago

  • Component changed from Menus to General
  • Keywords needs-patch added; reporter-feedback removed

i am using the hook wp_nav_menu_item_custom_fields

<?php
add_action( 'wp_nav_menu_item_custom_fields', 'theme_add_custom_fields_to_menu', 10, 4);

Please set a global parameter or a way to check that the action has been called already by wp !! as before wp 5.4 wp does not call that action and the theme and plugin developers have to call the action themselves !

Now i have to check the version and call the action. This is a bit weird.

<?php
class Theme_Walker_Nav_Menu_Edit extends Walker_Nav_Menu_Edit {
        
    function start_el( &$output, $item, $depth = 0, $args = array(), $id = 0 ) {
        $item_id = esc_attr( $item->ID );

        parent::start_el($output, $item, $depth, $args);

        ob_start();
        do_action( 'wp_nav_menu_item_custom_fields', $item_id, $item, $depth, $args );          
        $custom = ob_get_clean();                       

        $desc_snipp = '<div class="menu-item-actions description-wide submitbox">';

        $pos = strrpos( $output, $desc_snipp );
        if( $pos !== false ) {
            $output = substr_replace($output, $custom . $desc_snipp, $pos, strlen( $desc_snipp ) );
        }
    }
}

add_action( 'wp_nav_menu_item_custom_fields', 'theme_add_custom_fields_to_menu', 10, 4);

function theme_add_custom_fields_to_menu( $item_id, $item, $depth, $args ) {
    $custom = icon_settings($item_id);
    $custom .= animation_settings($item_id,$depth);
    $custom .= multicolumn_settings($item_id, $depth);
    $custom .= mobile_show_settings($item_id);
    $custom .= conditional_visibility($item_id);
    echo $custom;
}

}}}

each function adds the fields in the correct format as wp requires in the various tutorials.

Last edited 4 years ago by BackuPs (previous) (diff)

#4 @BackuPs
4 years ago

Hi

Its because a new action call has been added in wp 5.4. There is no way to check if it has been called already. Before wp 5.4 we had to call it ourselves as the action call in default wp is not there.

https://cl.ly/8653ce8bab3c

<?php

                                <?php
                                /**
                                 * Fires just before the move buttons of a nav menu item in the menu editor.
                                 *
                                 * @since 5.4.0
                                 *
                                 * @param int      $item_id Menu item ID.
                                 * @param WP_Post  $item    Menu item data object.
                                 * @param int      $depth   Depth of menu item. Used for padding.
                                 * @param stdClass $args    An object of menu item arguments.
                                 * @param int      $id      Nav menu ID.
                                 */
                                do_action( 'wp_nav_menu_item_custom_fields', $item_id, $item, $depth, $args, $id );
                                ?>

#5 @audrasjb
4 years ago

  • Component changed from General to Menus

#6 follow-up: @BackuPs
4 years ago

is this new action call gonna stay in wp 5.4?

if so i can check the wp version and do the call myselves if it is below wp version 5.4.

#7 @audrasjb
4 years ago

Related: #47056

#8 in reply to: ↑ 6 @SergeyBiryukov
4 years ago

Replying to BackuPs:

is this new action call gonna stay in wp 5.4?

Yes, see [47190] and [47350]. A dev note for the new hooks will be published on https://make.wordpress.org/core/.

if so i can check the wp version and do the call myselves if it is below wp version 5.4.

Sounds good, that would resolve the issue.

#9 @BackuPs
4 years ago

OK i will do so. Then close this topic please.

#10 @audrasjb
4 years ago

  • Keywords needs-patch removed
  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from reviewing to closed

Thanks for the ticket, @BackuPs! 👍

#11 @jnylen0
4 years ago

I think this should be reopened as the same issue will probably affect a lot of other plugins. See https://make.wordpress.org/core/2020/02/25/wordpress-5-4-introduces-new-hooks-to-add-custom-fields-to-menu-items/#comment-38016

Last edited 4 years ago by jnylen0 (previous) (diff)

#12 @BackuPs
4 years ago

@jnylen0

This was exactly the reason of my question, but the fix is really simple to apply. It should have been part of the wp core a long time ago.

https://make.wordpress.org/core/2020/02/25/wordpress-5-4-introduces-new-hooks-to-add-custom-fields-to-menu-items/#comment-38017

Core Team : Please let me know if the somehow the core team decides to change their action call as i already have adjusted the code to make the theme i use work.

#13 follow-up: @jnylen0
4 years ago

The fix in the WP code is also very easy: remove the wp_ prefix from the new hooks.

Otherwise, literally hundreds of plugin developers will have to make this change, and any custom sites that were also using this patch.

True, it's not a difficult change for each individual developer to make, but I don't see a good reason to require this.

#14 in reply to: ↑ 13 @BackuPs
4 years ago

Replying to jnylen0:

Otherwise, literally hundreds of plugin developers will have to make this change, and any custom sites that were also using this patch.

True, it's not a difficult change for each individual developer to make, but I don't see a good reason to require this.

Doesn't that apply to many of the changes wordpress made in the last 10 years? You don't want to know how often theme developers had to change their coding because of these and many plugin developers. Look at every woocommerce update. It is even worse then wordpress updates with its impact on plugin and theme developers. This is not different.

Note: I already asked if this is gonna be reverted and the answer was "no". See above reply from Sergey.

Actually the solution is even simpeler. I will just rename my add_action and do_action in which case i dont have to check for the wp_version.

Last edited 4 years ago by BackuPs (previous) (diff)

#15 follow-up: @helgatheviking
4 years ago

@BackuPs if you do this:

Actually the solution is even simpeler. I will just rename my add_action and do_action in which case i dont have to check for the wp_version.

Then you might recreate the issue that this hook was meant to solve... single Walker and fields from competing Walkers not displaying. I've seen plenty of themes using the menu Walker and people contacting me thinking my plugin wasn't working since my fields didn't display. having this hook as a community hook as greatly reduced that and getting it in to core should make it go away entirely.... assuming all theme/plugin devs do a version check and stop switching the Walker as of WP 5.4.

Something like this should do it and it's what I'll be adding to my plugin soon.

global $wp_version;
if( ! version_compare( $wp_version, '5.4-RC4', '>=' ) ) {
	add_filter( 'wp_edit_nav_menu_walker', 'helga_switch_nav_menu_walker' );
}

#16 in reply to: ↑ 15 @BackuPs
4 years ago

Replying to helgatheviking:

Something like this should do it and it's what I'll be adding to my plugin soon.

global $wp_version;
if( ! version_compare( $wp_version, '5.4-RC4', '>=' ) ) {
	add_filter( 'wp_edit_nav_menu_walker', 'helga_switch_nav_menu_walker' );
}

I am sorry but your code used is not save !

it should be like this as it should work on any wp version not only beta.

if (version_compare(preg_replace("/[^0-9\.]/","",$wp_version), '5.4', '<') ) {
}

alss has been explained and discussed here https://make.wordpress.org/core/2020/02/25/wordpress-5-4-introduces-new-hooks-to-add-custom-fields-to-menu-items/

Last edited 4 years ago by BackuPs (previous) (diff)

#17 follow-up: @helgatheviking
4 years ago

Not sure I understand why that wouldn't be safe? In my production code I'm using version_compare( $wp_version, '5.4' )... the RC is only for testing locally. But if someone is using a beta/RC on a live site... well they will see the duplicates. I'm not gonna sweat that too much.

#18 in reply to: ↑ 17 @BackuPs
4 years ago

Replying to helgatheviking:

Not sure I understand why that wouldn't be safe? In my production code I'm using version_compare( $wp_version, '5.4' )... the RC is only for testing locally. But if someone is using a beta/RC on a live site... well they will see the duplicates. I'm not gonna sweat that too much.

because it will fail on versions numbers (beta) that are not numerical ! You dont know if your users are using beta versions of wordpress. The choice is yours but it is asking for future troubles if you use it the way you do.

#19 follow-up: @helgatheviking
4 years ago

If users are using a beta on a live site then that's on them. But using strtolower does seem to get version comparison to work without needing a preg_replace

ex: version_compare( strtolower( $wp_version ), strtolower( '5.4-RC1' ), '>=' )

Last edited 4 years ago by helgatheviking (previous) (diff)

#20 in reply to: ↑ 19 @BackuPs
4 years ago

Replying to helgatheviking:

If users are using a beta on a live site then that's on them. But using strtolower does seem to get version comparison to work without needing a preg_replace

ex: version_compare( strtolower( $wp_version ), strtolower( '5,4-RC1 ), '>=' )

It is working now because you are using a beta release. The moment you dont it won't work.

Goodluck with that in the future wp builds. It is weird php coding you are using !

I will stop replying on this useless discussion

Last edited 4 years ago by BackuPs (previous) (diff)

#21 @helgatheviking
4 years ago

version_compare() is straight out of the PHP manual:
https://www.php.net/manual/en/function.version-compare.php

so while I have some typos above, once I fix that version 5.4 will be greater than 5.4-RC1. Not even sure I still need the strtolower anymore once I fix my typos.

$wp_version = '5.4';

$result = version_compare( strtolower( $wp_version ), strtolower( '5.4-RC1' ), '>=' );

var_dump($result); // true

Can you give me an example where this will not work?

Last edited 4 years ago by helgatheviking (previous) (diff)
Note: See TracTickets for help on using tickets.