WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#12715 closed defect (bug) (fixed)

wp_get_nav_menu_item runs esc_html on attributes

Reported by: pdclark Owned by: ptahdunbar
Milestone: 3.0 Priority: normal
Severity: normal Version: 3.0
Component: Menus Keywords: nav menu, nav-menus, navigation, menus
Focuses: Cc:

Description

In wp-includes/nav-menu-template.php, the function wp_get_nav_menu_item() runs esc_html on the attributes passed to the menu in line 155 through 159.

The specific attributes are before, after, link_before, and link_after.

This may be a "feature," not a bug, but below is the use case where I consider the use of esc_html to be in error:

Say I create a menu, and for styling purposes, would like to wrap each link in a DIV:

$atts = array(
	'container_class' => 'menu-header',
	'before' => '<div>',
	'after' => '</div>',
	'menu' => '',
);

wp_nav_menu( $atts );

Expected output for a link for this would be:

<li><div><a href="#">Link</a></div></li>

Instead, esc_html turns it into:

<li>&lt;div&gt;<a href="#">Link</a>&lt;/div&gt;</li>

Which causes those <div> tags to be output as actual text, rather than being used for markup.

Working around the esc_html gets a bit hacky, with one solution looking like this:

$atts = array(
	'container_class' => 'menu-header',
	'echo' => false,
	'before' => '%div%',
	'after' => '%/div%',
	'menu' => '',
 );

wp_nav_menu( $atts );
$menu = str_replace(
	array( '%div%', '%/div%',),
	array( '<div>', '</div>',),
	$menu
);

echo $menu;

It seems to me that it might make more sense to apply esc_html via a filter, so it can be added or removed in a more standard way, or to do away with it all together, assuming that the before and after attributes are going to often be used for HTML markup.

Attachments (1)

remove_esc_html_nav_menus.diff (908 bytes) - added by ptahdunbar 10 years ago.

Download all attachments as: .zip

Change History (4)

#1 @nacin
10 years ago

  • Component changed from General to Menus
  • Milestone changed from Unassigned to 3.0
  • Owner set to ryan

Sounds like it should be removed.

#2 @ptahdunbar
10 years ago

  • Owner changed from ryan to ptahdunbar
  • Status changed from new to assigned

I got it.

#3 @markjaquith
10 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [14013]) These nav_menu parameters should not be esc_html()d. fixes #12715. props ptahdunbar

Note: See TracTickets for help on using tickets.