Possible problem with translated content types in the settings form

I think you likely spotted the problem correctly. Thank you! I'll be looking at this in the next couple of days. I know it's been a while, so if you happen to have a patch you've already tried, please throw it on here and I'll look to roll it in.

I think you likely spotted the problem correctly. Thank you! I'll be looking at this in the next couple of days. I know it's been a while, so if you happen to have a patch you've already tried, please throw it on here and I'll look to roll it in.

We were inconsistently translating things like module names in the include files; internal naming of modules should probably never be run through t() like we were doing. Patch enclosed.

I'm not too sure that this change is valid:

         $types[] = array(
           'name' => $type->type,
           'module' => $type->module,
-          'title' => $type->name,
-          'description' => $type->description,
+          'title' => t($type->name),
+          'description' => t($type->description),
           'default_value' => 0,
         );
       }

Core doesn't seem to ever apply t() to the name and description of a node type. (which I think is why they have that separate translation module.)

There is one exampled in modules/node/node.module around line 1737:

    $items['node/add/'. $type_url_str] = array(
      'title' => drupal_ucfirst($type->name),
      'title callback' => 'check_plain',
      'page callback' => 'node_add',
      'page arguments' => array(2),
      'access callback' => 'node_access',
      'access arguments' => array('create', $type->type),
      'description' => $type->description,
      'file' => 'node.pages.inc',
    );

Thank you.
Alexis

That was done for consistency. If you notice our other content type modules, they all t() their names and descriptions.

The reason this is done is because the function call is substantively different from the calls you're noting in the core modules. Those are defining menu items and other such system-wide items. We're simply enumerating options to go into a checkbox list.

Though I did it for consistency, I guess we could just change all the content types to return straight strings and t() the strings in node.module -- the end result would be the same. Do you have a preference?

Ah! Good point! The menu actually does the t() call for you on the name... so I guess I'm wrong on that one. However, I do not think they do it on the description.

In general, I think it is better to allow for translations. I was just surprised by the addition of a couple t() calls. I don't know the code as well as you and did not know that the other sub-include were using it already.

I think that the current patch is fine then.

Thank you.
Alexis

zeropaper, could you possibly test this patch and make sure you don't see any unintended side effects?

Gnassar,

You have quite high hopes here... He posted that note in March... 8-)

Thank you.
Alexis

Looks good to me.

Love me some RTBC!

Committed.
http://drupal.org/cvs?commit=470256
http://drupal.org/cvs?commit=470258