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.

CommentFileSizeAuthor
#7 D6_advanced_help_translatability_bugfix.patch606 byteshass
D6_advanced_help_translatability_bugfixes.patch3.15 KBhass

Comments

merlinofchaos’s picture

merlinofchaos commented
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

hass commented

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

hass commented

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

hass commented

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

Freso commented

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

merlinofchaos commented
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

hass commented
Title: Translatable string bugs » Translatable string bug
Status: Fixed » Reviewed & tested by the community
StatusFileSize
new D6_advanced_help_translatability_bugfix.patch606 bytes

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

merlinofchaos’s picture

merlinofchaos commented
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

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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