Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
In i18n_menu there's a need to bypass t() since translation is handled without that, however this only appears to be possible if you set title_callback to a dummy function (in this case return $title) - seems like it would be useful to have a flag, so that you can simply return the raw title if that's necessary - something like NO_CALLBACK?
Comment | File | Size | Author |
---|---|---|---|
#14 | drupal-menu-title_callback.patch | 4.04 KB | catch |
#12 | drupal-menu-title_callback_1.patch | 4.24 KB | stella |
#9 | drupal-menu-title_callback.patch | 4.24 KB | catch |
#8 | drupal-menu-title_callback.patch | 1.58 KB | catch |
#2 | drupal-menu-title_callback.patch | 692 bytes | smk-ka |
Comments
Comment #1
Damien Tournoud CreditAttribution: Damien Tournoud commentedSetting
title callback
to 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 CreditAttribution: 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 CreditAttribution: 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 CreditAttribution: Damien Tournoud commentedThis restores a condition that was in D6, but was accidentally removed during the conversion to drupal_function_exists().
Comment #6
Dries CreditAttribution: 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 CreditAttribution: 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.