Posted by roball on May 6, 2011 at 3:55pm
4 followers
| Project: | Menu Breadcrumb |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
The Menu HTML module allows to add HTML to a menu item's name, which is usually used to display an icon next to the name, like this:
Menu link title <img src="/sites/mysite/files/myicon.png" />When the "Append page title to breadcrumb" option is checked, the menu name will be determined by drupal_get_title(), which adds a check_plain() and thus destroys the angle brackets around HTML tags and a literal " character. Instead of the icon being displayed within the last piece of the breadcumb, the HTML source is displayed. To restore the image being displayed, menu_breadcrumb.module has to be changed as follows:
Change
if (variable_get('menu_breadcrumb_append_node_title', 0) == 1) {
if (variable_get('menu_breadcrumb_append_node_url', 0) == 1) {
$breadcrumb[] = l(drupal_get_title(), $_GET['q'], array('html' => TRUE));
}
else {
$breadcrumb[] = drupal_get_title();
}
}(beginning on line 340) to
if (variable_get('menu_breadcrumb_append_node_title', 0) == 1) {
// drupal_get_title() adds check_plain() => angle brackets around HTML tags get lost => restore them
$node_title = drupal_get_title();
$node_title = preg_replace('/</', '<', $node_title);
$node_title = preg_replace('/>/', '>', $node_title);
$node_title = preg_replace('/"/', '"', $node_title);
if (variable_get('menu_breadcrumb_append_node_url', 0) == 1) {
$breadcrumb[] = l($node_title, $_GET['q'], array('html' => TRUE));
}
else {
$breadcrumb[] = $node_title;
}
}Would you consider to commit this change? Thanks.
Comments
#1
Feels like this ought to be done in the site theme, but I appreciate that for some people and sites a tool like Menu HTML may be a better solution so will consider if a patch against 6.x-1.x is supplied.
#2
Actually, looking at the code above I think it will need to be better than that.
The preg_replace() in your description won't fully restore HTML, just reverse the HTML encoding of some characters ... that doesn't feel so good.
I don't think I'd commit the code above, no. You're welcome to continue working on this and/or raise some support for the idea if you think I'm wrong!
#3
OK, so let's better get totally rid of HTML that may be within a menu item title, probably specified via the Menu HTML module. Using
menu_get_active_title()instead ofdrupal_get_title()keeps the HTML, which can then be stripped out by the following code (replacing my first attempt above):<?phpif (variable_get('menu_breadcrumb_append_node_title', 0) == 1) {
$node_title = menu_get_active_title();
$node_title = preg_replace('/<[^>]*>.*$/', '', $node_title);
if (variable_get('menu_breadcrumb_append_node_url', 0) == 1) {
$breadcrumb[] = $is_front ? l(t('Home'), '<front>') : l($node_title, $_GET['q'], array('html' => TRUE,));
}
else {
$breadcrumb[] = $is_front ? t('Home') : $node_title;
}
}
?>
I have successfully tested it. Thanks for considering.
#4
#5
Attached is the proper patch against the latest commit of the 6.x-1.x branch.
#6
#7
Rather than preg_replace(), do you think filter_xss($node_title, array()) or even strip_tags($node_title) might be a better choice?
filter_xss() runs several preg_replace, but makes sense as it re-uses existing Drupal functions rather than trying to roll our own regex tag stripper. Looks like filter_xss() catches a few more cases than the regex in the patch too, eg <title>this <unclosed tag</title>.
#8
Here's the patch for D7, with filter_xss suggestion included.
#9
Thanks, committed to 6.x-1.x and 7.x-1.x.
#10
Thank you grobot for committing. However, you should change the 6.x-1.x-dev release to be built based on the 6.x-1.x branch (instead of master), so the commit will be available in the latest auto-built 6.x-1.x-dev snapshot.
#11
Thanks for spotting that roball. Have updated the branch for that release now - I expect the release bot should pick up on this shortly.
#12
Can confirm that Menu Breadcrumb 6.x-1.3+5-dev (2012-Apr-18) fixes the reported problem, especially it now also works fine together with the "Menu HTML" module. Thanks again for committing.
#13
Automatically closed -- issue fixed for 2 weeks with no activity.