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?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Damien Tournoud’s picture

Setting 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.

smk-ka’s picture

Status: Active » Needs review
FileSize
692 bytes

Setting title callback to FALSE will probably work.

This 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.

Damien Tournoud’s picture

@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.".

nedjo’s picture

For 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().

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community

This restores a condition that was in D6, but was accidentally removed during the conversion to drupal_function_exists().

Dries’s picture

Status: Reviewed & tested by the community » Needs work

Is the "Setting title callback to FALSE" approach documented and tested? It sounds like we need to document this a little bit better ...

nedjo’s picture

Issue tags: +i18n sprint
catch’s picture

Status: Needs work » Needs review
FileSize
1.58 KB

Both '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.

catch’s picture

Now with a test.

Status: Needs review » Needs work

The last submitted patch failed testing.

catch’s picture

Status: Needs work » Needs review

HEAD broken.

stella’s picture

Re-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.

Status: Needs review » Needs work

The last submitted patch failed testing.

catch’s picture

Status: Needs work » Needs review
FileSize
4.04 KB

Re-rolled.

nedjo’s picture

Status: Needs review » Reviewed & tested by the community

A quick summary of the issue and why I'm marking it RTBC:

Issue:

  • In some cases modules may wish to have no callback for menu titles, instead returning the title unmodified. There is a use case in i18nmenu module.
  • Currently it works to simply set 'title callback' to FALSE and get the desired result, but this feature is undocumented, and inefficient in that it calls drupal_function_exists() unnecessarily.
  • In fact there is no documentation for title callback.

Patch:

  • Explicitly tests for a callback before invoking drupal_function_exists(). This makes the feature more discoverable and prevents the unnecessary drupal_function_exists() call.
  • Correctly documents the title callback argument, including the use of FALSE.
  • Includes a test that correctly ensures a FALSE title callback returns the original title unaltered.
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks!

sun’s picture

Version: 7.x-dev » 6.x-dev
Status: Fixed » Patch (to be ported)

If I'm not mistaken, the original intent was to fix 6.x, to allow i18nmenu to work properly.

catch’s picture

It 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.

sun’s picture

Status: Patch (to be ported) » Fixed

Committed the PHPDoc fix to developer docs DRUPAL-6--1.

Status: Fixed » Closed (fixed)
Issue tags: -i18n sprint

Automatically closed -- issue fixed for 2 weeks with no activity.