How to make hook_search_info() titles translatable needs docs

I think it's not supposed to be translated since the main place it's used is hook_menu() (see https://api.drupal.org/api/drupal/modules!search!search.module/function/...) - titles in hook_menu() should not be translated since that would cause the menu system to cache the title in a particular language. Rather, they get run through t() on display.

However if there's an issue with the translation extractor not picking the hook_search_info() titles up for translation at all, or if they are being displayed somewhere else where the lack of translation is a problem, then there might need to be something done to fix this.

So based on the above the patch doesn't seem correct as is (and not complete either, since this isn't limited to the Node module's implementation) but I'm moving it to "needs review" and to the Search module area of the issue queue where people more familiar with this might be able to review it. Thanks!

Interesting issue report, thanks!

So. I checked all the invocations of hook_search_info(), and the only one that uses the 'title' component is in search_menu() (the Search module's hook_menu() implementation). So David is correct, we would not want the 'title' to be translated in hook_search_info(), because 'title' is not supposed to be translated in hook_menu().

But you are right that the POT extractor is probably not noticing that this needs to be added to the translation database.

So. What I think we should do on this issue is to document, in search.api.php, that the 'title' returned should be untranslated, but that in your function, you should also make a call to t() to translate that string, so that it gets added to the translation database.

So, this patch should:
- Add this information to the hook_search_info() docs
- Add the t() call to the sample function body in the hook_search_info() docs

Fixing title, and I think this would be a good Novice project in Documentation. Adding novice task to summary.

Going to look into this.

As already mentioned, the 'title' from hook_search_info() can't have the t() function to not implicate on search_menu(). Considering that, I've added a comment that it should be translatable in case of a need. Also, I've included the t() function to sample function body in the hook_search_info() docs.

About the node_search_info() changes, I've removed them, as the issue is related to hook_search_info() being translatable. Is that correct?

Thanks.

Thanks for the patch!

When you are making patches for an issue, PLEASE leave it assigned to yourself until it is done. It is very annoying when multiple people come and go on issues, and leaving it assigned indicates you will (hopefully) come back and fix any problems with the patch. Thanks!

So, the patch isn't quite right. You are correct that we do not want the change to node_search_info(). However, you've made the same mistake in this patch that we *don't* want implementations of this hook to do:

1. +++ b/modules/search/search.api.php
   @@ -31,7 +31,8 @@
   + *     to the module name if not given. Should be untranslatable, but in case of
   + *     need, should be passed through the t() function.
   

   This is misleading.

   First, it's not "untranslatable". It's "untranslated". And it *must* be, not *should*.

   Second, we need to clarify that in order to make it translatable, you need to pass it through the t() function but it must be somewhere else, not in the 'title' element.

2. +++ b/modules/search/search.api.php
   @@ -40,7 +41,7 @@
   +    'title' => t('Content'),
   

   No. Again, we *cannot* put t() in this line. The string in this line cannot be translated.

   What we need instead is that before the return line, a new line is added that says:

   $x = t('Content');
   

   along with a // comment before that line that explains why we are doing this.

I'm not entirely sure I understand the second part. Please, let me know if there's anything else I should do.

Good try, but still that's not quite right.

So what you'd want in the function body is something like this:

function hook_search_info() {
  // Call t() to get the search title into the translation database.
  $x = t('Content');
  return array(
     'title' => 'Content',
...

Then in the documentation, we should say for 'title' something like:
title: Untranslated title for ...

That thing you put in for 'translated_title' needs to go away. It will not be processed by Drupal in any way.

Thanks!

Got it @jhodgdon.

I've sent the 'title' element content through the t() function. Also, I've tested without assigning to a variable and it worked. Is that correct? I tried to find any similar implementation or standard, but couldn't find it.

Thanks for the new patch! You're right, there is no need to assign the output of t() to a variable.

However... I think this still has some problems:

1. +++ b/modules/search/search.api.php
   @@ -31,7 +31,9 @@
   + *     to the module name if not given. Must be untranslated, but in case of
   + *     need, the content must be sent through the t() function to register it
   + *     within translation database.
   

   I don't understand "in case of need". This is in a module, and you don't know who's going to be using the module on a non-English site when you develop a module. So, every module should run the title through t().

   How about just saying:

   Must be untranslated. Outside of this return array, pass the title through the t() function to register it as a translatable string.

   Or something like that?

2. +++ b/modules/search/search.api.php
   @@ -39,6 +41,9 @@
   +  // In case the 'title' element content needs to be translated.
   

   Again, there's no "in case". It just needs to be translatable.

   Maybe make the comment say something like:

   Make the title translatable.

@jhodgdon
Updating comment text as mentioned by you.

I will go with hook_form_FORM_ID_alter() and add t() function for translatable.

Well we should fix the node_search_info() and user_search_info() functions to do this same thing. I do not think that your hook_form_*_alter() function will make this translatable, especially since the tab title is not part of a form.

The patch in #14 looks good, thanks! I guess we should also fix this in those two Core implementations of the hook? And it may need a test... so maybe not just Documentation and maybe not Novice.

Although... probably the Node/User implementations are OK because they are using strings that are translated in Core already: 'Content' and 'Users'. So this is probably fine as it is. Thanks!

Committed to 7.x - thanks!