This patch fixes some bugs where strings are not shown translated in UI and fixes two context sensitive bugs.

It does not fix the bug in the menu title. POTX complains about the module_exist() construct. We should change this.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

Status: Needs review » Needs work

Hmm. This patch seems to go a little far. While the missing t() around 'Help' is bad (can't believe no one's caught that before now) is a no-brainer, you've got lots of little translatable strings that come out of the .ini file; the .ini file is translateable directly. I'm not sure that these variable strings should be run through t() like that.

I'm afraid I don't know what you're referring to with the module_exists construct.

hass’s picture

Ups, I wasn't aware that some of this strings came out of the .ini files. I'm not sure about the combination... partly you take some out of the .ini, but partly not. I'm only guessing now - but $items[] = advanced_help_l(t($name), "admin/advanced_help/$module"); and advanced_help_l(t(advanced_help_get_module_name($pmodule)), "admin/advanced_help/$pmodule"); seems not inside the .ini files and is therefore correct, isn't it?

If the DE translation would work I could test better... but I was unable to get this working in cck and advanced_help, yet.

POTX complains on this construct:

$items['admin/advanced_help'] = array(
    'title' => module_exists('help') ? 'Advanced help' : 'Help',
hass’s picture

Additional for t('<< @topic_title', array('@topic_title' => $topics[$prev[0]][$prev[1]]['title'])) it seems like we need to rebuild the t() function into a advanced_help_t().

hass’s picture

Ok, as I've used a wrong folder for the html files this seems now working. But on "admin/advanced_help" the list items are not translated as they are not inside the .ini files. Not sure how we should fix the list building function as there is a mix of .ini file strings and core strings!?

Freso’s picture

One other comment:

@@ -200,7 +200,7 @@
         else {
          $name = t($module_name);
         }
-        $items[] = advanced_help_l($name, "admin/advanced_help/$module");
+        $items[] = advanced_help_l(t($name), "admin/advanced_help/$module");
       }
     }
 

I don't know what's above the else {, but if it sets $name like the else does (or it is set in the else), the t($name) will translate an already translated string. Which could quickly and easily become a mess...

merlinofchaos’s picture

Status: Needs work » Fixed

I've committed the missing t() fixes; the rest I think are unnecessary unless I've missed something. WHich I may have, but they're fixed by making sure that they're pulled from the translated .ini file; let's do separate issues for that if they are a problem.

hass’s picture

Title: Translatable string bugs » Translatable string bug
Status: Fixed » Reviewed & tested by the community
FileSize
606 bytes

We missed one "Home" string. Here is a patch. Sooo trivial that I'm setting RTBC.

merlinofchaos’s picture

Status: Reviewed & tested by the community » Fixed

Apparently this got in in the next commit (the patch was against 1.31 and 1.32 has the change already).

Anonymous’s picture

Status: Fixed » Closed (fixed)

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