This may indicate a more general problem, but this solves the issue for content type names as menu item titles.

For example, my content type called 'Q&A' appears in the Manage menu block as 'Q&A'

The attached patch replaces check_plain with filter_xss($name, array()) -- i.e., filter_xss() + strip_tags().

CommentFileSizeAuthor
#2 node_menu.01.patch775 bytesmatt2000
node_menu.patch765 bytesmatt2000

Comments

Status: Needs review » Needs work

The last submitted patch failed testing.

matt2000’s picture

matt2000 commented
Status: Needs work » Needs review
StatusFileSize
new node_menu.01.patch775 bytes

Good Testbot. Here's a jelly bean. ;-)

Status: Needs review » Needs work

The last submitted patch failed testing.

matt2000’s picture

matt2000 commented
Status: Needs work » Needs review

Bad Testbot! everything passed locally... &-(

heine’s picture

heine commented
Status: Needs review » Needs work

This is indeed a more general problem.

We must decide whether the title received from the title callback is HTML or should be treated as plaintext. In the former case, the correct title for the OP would be Q&A and drupal_get_title should not check_plain the result from menu_get_active_title(), nor should any function printing menu titles use check_plain on them. The one declaring the menu then needs to take care of escaping plaintext values.

Should we decide menu titles are plaintext, we can keep current behaviour, but document the fact and tell people t('%placeholders') won't work anymore for menu titles.

heine’s picture

heine commented
Title: check_plain in node_menu mangles perfectly good titles » Are menu titles plaintext or HTML

Updating title to reflect the general issue.

heine’s picture

heine commented

... we can keep current behaviour ...

Apart from solving the obvious double encoding issue here :)

matt2000’s picture

matt2000 commented

If people want to use %placeholders, couldn't they do 'title callback' => 't' ? Or is that a misuse of the t() function?

heine’s picture

heine commented

@matt2000, we need to decide how we treat the return value of the title callback, regardless of what the callback is.

(Now, if we had the ASP.NET 4's IHtmlString workaround, we could return whatever we wanted)

matt2000’s picture

matt2000 commented

OK, I get it now.

If we've explicitly or implicitly allowed HTML titles up until now, I think it would be too drastic a change to force plain text now. We should figure out a way to allow it.

drupal_was_my_past’s picture

drupal_was_my_past commented
Status: Needs work » Closed (duplicate)

Marking this as a duplicate of #1188394: Make title behave as a configurable, translatable field (again).

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Aforementioned issue is not related here.

However, this issue seems indeed to be a duplicate of #556910: Menu router items: Allow to pass PASS_THROUGH to t()