Closed (fixed)
Project:
Drupal core
Version:
6.x-dev
Component:
menu system
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
18 Dec 2008 at 12:17 UTC
Updated:
2 Jan 2014 at 23:45 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
damien tournoud commentedSetting
title callbackto FALSE will probably work. But we could optimize how_menu_item_localize()works to avoid the call to drupal_function_exists() in that case.Comment #2
smk-ka commentedThis feature was part of D6, but seems to have been removed in HEAD. I'm providing a patch to put it back in, unless someone explains why it has been removed.
Comment #3
damien tournoud commented@smk-ka: passing FALSE to drupal_function_exists() will return FALSE, too, so the feature is on HEAD :) Your patch fix the second part of my comment in #1: "But we could optimize how _menu_item_localize() works to avoid the call to drupal_function_exists() in that case.".
Comment #4
nedjoFor i18nmenu, the need probably is not to bypass translation but to be able to get contextual information (a menu id) in the title callback.
See this related patch: #348555: ID information needed for translating custom menu items.
Are there any cases in which check_plain() would be an inappropriate fallback if no title callback is set? There is already special handling for check_plain() in _menu_item_localize().
Comment #5
damien tournoud commentedThis restores a condition that was in D6, but was accidentally removed during the conversion to drupal_function_exists().
Comment #6
dries commentedIs the "Setting title callback to FALSE" approach documented and tested? It sounds like we need to document this a little bit better ...
Comment #7
nedjoComment #8
catchBoth 'title callback' and 'title arguments' were undocumented in the menu.api.php docs for hook_menu() - attached patch adds that in.
I'll also add a note to http://drupal.org/node/140311 when this is committed.
This doesn't actually change existing behaviour, just makes it more self-documenting and avoids a registry hit. However will think about ways to add a concise test for this to the hook_menu tests, I guess we could have a title with a @placeholder in, pass FALSE as title callback, then add superfluos title arguments which would show up if it's run through t(), and confirm that the raw string is output? Can't think of an easy way to test for the absence of t() otherwise.
Comment #9
catchNow with a test.
Comment #11
catchHEAD broken.
Comment #12
stella commentedRe-rolled the patch to fix a minor typo in function comment description - that's the only change.
New documentation looks good and all tests pass.
Comment #14
catchRe-rolled.
Comment #15
nedjoA quick summary of the issue and why I'm marking it RTBC:
Issue:
Patch:
Comment #16
webchickCommitted to HEAD. Thanks!
Comment #17
sunIf I'm not mistaken, the original intent was to fix 6.x, to allow i18nmenu to work properly.
Comment #18
catchIt was, but we subsequently found out it was a documentation issue rather than an actual bug (+ the drupal_function_exists() in 7). We should probably backport the phpdoc though.
Comment #19
sunCommitted the PHPDoc fix to developer docs DRUPAL-6--1.