Walker_Nav_Menu filter nav_menu_link_attributes checks $atts argument via empty() and not isset(), causing confusion with legitimate values that evaluate to boolean false

[nevma]

In file wp-includes/class-walker-nav-menu.php, line 206, class function start_el(), where the nav_menu_link_attributes filter is applied, the current code checks the supplied attributes via PHP empty() function.

This is the current code:

foreach ( $atts as $attr => $value ) {
    if ( ! empty( $value ) ) {
        $value       = ( 'href' === $attr ) ? esc_url( $value ) : esc_attr( $value );
        $attributes .= ' ' . $attr . '="' . $value . '"';
    }
}

If an attribute with a legitimate value, which evaluates to something that equals to boolean false is passed, then the if clause will not be executed. However, there are many cases where such values are useful.

For instance an HTML data attribute with a value of zero (0) could be passed as an attribute in the $atts array, but its value would then be equal to boolean false (as far as PHP empty() is concerned and thus the value would be erroneously ignored.

I believe that this check would be better done with the PHP isset() function like this:

foreach ( $atts as $attr => $value ) {
    if ( ! isset( $value ) ) {
        $value       = ( 'href' === $attr ) ? esc_url( $value ) : esc_attr( $value );
        $attributes .= ' ' . $attr . '="' . $value . '"';
    }
}

[AkSDvP]

Key differences between both methods is-

isset - Determine if a variable is set and is not NULL.
!empty - Determine whether a variable is NOT empty.

Considering the above point I have replaced
if ( ! empty( $value ) )
with
if ( isset( $value ) )

[greenshady]

Just ran into this problem myself.

In a similar attribute system I built, here's how I handle it, which also allows attributes without a value (where the value would be the same as the attribute name):

foreach ( $this->all() as $name => $value ) {

	$esc_value = '';

	// If the value is a link `href`, use `esc_url()`.
	if ( $value !== false && 'href' === $name ) {
		$esc_value = esc_url( $value );

	} elseif ( $value !== false ) {
		$esc_value = esc_attr( $value );
	}

	$html .= false !== $value ? sprintf( ' %s="%s"', $name, $esc_value ) : " {$name}";
}

[davidbaumwald]

@SergeyBiryukov Is this still realistic for 5.3? If not, feel free to move to Future Release or 5.4 based on your preference.

[SergeyBiryukov]

In 46413:

Menus: In Walker_Nav_Menu, Walker_Category, and Walker_Page, properly output link attributes having a legitimate "empty" value, for example an HTML data attribute with a value of zero (0).

Props nevma, AkSDvP, greenshady, SergeyBiryukov.
Fixes #47720.