Translation of messages menu breaks display of unread messages

Firestly, are you using 6.x-1.x-dev or HEAD?

You should be using HEAD as the former is a dead end.

Secondly, I am not sure what you mean in the above post. menu items for drupal 6 should not have a t() function. The translation stuff is done in a different manner (not sure how.)

Menuitem's titles and description should not have a t() function in drupal 6
Take a look at http://drupal.org/node/103114

hi,

i hope its no duplicate, when i rename the menuitem from messages to "nachrichten" there is no indication if there is a new message or not ( the (1) behind the menuitem). A translation with the translation options from drupal isn't possible (can't find any "messages" that is not translated anymore).

Thanks

strauch

The drupal 5 version of privatemsg got around this by having an option in the privatemsg admin settings which would rename the menu item.

Would translations work with our current approach or do we have to go that way?

I'm setting this one to active again...

Maybe there is a problem with title callback and translation caching or something. I think we should simply set the name of the menu item explicitly with an t()-call in the title_callback function instead of using the existing name. Similiar to what we are doing in the block.

Here is a patch that does what I mentioned above. Instead of relying on $title, it does return a translatable string.

but can the people rename the menu item with this approach?

If not, maybe have a setting allowing the admins to choose the text to show?

I would also assume the following will not work?

-  $title .= $new;
+  $title = t($title) . $new;

Yes, it does work for me, I was able to translate it to german.

Your example would work too, however t() should not be called with variables so that the extraction of translatable strings is possible, see http://drupal.org/project/potx.

My line of thought was that if the message menu item had not been renamed, then the string would have been translated... if it had been changed, then the admin probably does not wanted the translated string.

Does the translation stuff not work like that?

Good point, I can see what you mean. However, that's the actual issue here... *if* the menu item get's renamed, the title callback does not get called anymore, just tested it on my site. This simply means that if the callback is called, $title *is* what we defined in privatemsg_menu. So, the only advantage of $title is imho that we don't need to change 'Messages' at two places if we want to use a different string.

On the other hand, by using my patch, we could even think about changing the 'title' attribute to something like: "Messages **Warning: Do not change this or it will break the display of new messages**", because it is only displayed when the menu is edited. Just an idea...

Edit: Or we add, as you suggested, an option in the admin interface which allows to define a different text...

is it really privatemsg's responsibility to handle string translations beyond the basic use of translation api (ie using t() etc...) ?

In this case, IMO yes as the functionality is impaired/does not work when using translations.

I second that. The problem is that with defining a title callback, t() is not called automatically anymore and we need to do it manually.

Hello Berdir,

this patch works fine for me. Thank you.

Greetings

strauch

I set the status to reviewed and tested, i tested it and it works fine.

As statet at http://drupal.org/node/140311 (Case 3 and 4), using t() either in privatemsg_menu or in privatemsg_title_callback is perfectly fine and needed. Doing it in hook_menu would probably even be a bit faster.

You'll notice that we run the output here through t() ourselves, and we do it in chunks - part of it even in the title, where we generally aren't supposed to do it anymore! When our title callback displaces t() as the localizer, we have to either call it ourselves in our custom title callback, or else do localization in some other way.

If we are talking about patch #7... i am in favor of this solution with one minor modification. we need to change so the !count variable contains only number of new messages (0 - n). currently we add ( and ) around it and i think we shouldnt. perhaps some site maintainer would want their title to be 'Messages - N' rather than 'Messages (N)', etc.

Otherwise this is good :)

Updated :)

http://api.drupal.org/api/function/format_plural/6

We should use format_plural to handle the above :)

Sorry for being so difficult :(.

Was my idea too, but that doesn't work, afaik.

format_plural is hardcoded for 1 or 2-n. But we need 0 or 1-n.

bear in mind that current setup doesnt allow site owner to display title such as 'Messages (0)'. we should fix that if the issue comes up.

commited. thanks.