Support from Acquia helps fund testing for Drupal Acquia logo

Comments

spiffl’s picture

Thanks, that patch solved the issue for me.

mojiro’s picture

This patch works correctly, but the same problems exists on entity reference fields that use drupal's autocomplete widget.

Georgique’s picture

One more problem for file fields - description is not translated properly.

Georgique’s picture

Status: Active » Needs review
funature’s picture

I have this issue not just with File field type, but also with entityreference and geofield field types. After using this patch, the file field is ok now, but the others are still the same problem.

mojiro’s picture

I have noticed that multivalue entity reference are being translated correct.

funature’s picture

@mojiro, you are right, if I set them to multi-value, not only entity reference fields but also the Geofields are translated correct.

pbuyle’s picture

In addition to the missing title for field type where the $element contains the actual widget element under the 0 or value, the attached patch also add translations ofr their descriptions. It also fixes translations of list_boolean fields' labels.

mojiro’s picture

Unfortunately this patch did not work for entity reference fields where cardinality is 1.

I tried to figure out the diferences among text fields, entity reference fields with cardinality 1 and entity reference fields with cardinality -1 (unlimited), but nothing yet.

I have also upgraded i18n, entity, entity_reference and entity_translation to the latest dev versions but nothing yet.

By using these dev versions the first patch is not needed.

mojiro’s picture

I managed it to work! After applying the #8 patch, in file i18n_field/i18n_field.module

1st change

Find
if (empty($element['#entity']) && empty($element['value']['#entity']) && empty($element[0]['#entity'])) {

And Replace with
if (empty($element['#entity']) && empty($element['value']['#entity']) && empty($element[0]['#entity']) && empty($element['target_id']['#entity'])) {

2nd change

Find

if (isset($element[0]) && !empty($element[0]['#title']) && $element[0]['#title'] == $instance['label']) {
  $element[0]['#title'] = $instance_current['label'];
}

Add after

if (isset($element['target_id']) && !empty($element['target_id']['#title']) && $element['target_id']['#title'] == $instance['label']) {
  $element['target_id']['#title'] = $instance_current['label'];
}

3rd change

Find

if (isset($element[0]) && !empty($element[0]['#description']) && $element[0]['#description'] == $instance['label']) {
  $element[0]['#description'] = $instance_current['label'];
}

Add after

if (isset($element['target_id']) && !empty($element['target_id']['#description']) && $element['target_id']['#description'] == $instance['description']) {
  $element['target_id']['#description'] = $instance_current['description'];
}
attiks’s picture

FileSize
1.39 KB

Rerolled patch from #8 with remarks from #10

Still not working for field collections, but that's probably for an other issue

attiks’s picture

FYI: issue for field collection table at #1914568: Table headers aren't translated on edit screen

attiks’s picture

FileSize
4.26 KB

New patch sets the title on the value as well.

Status: Needs review » Needs work

The last submitted patch, i1904368-13.patch, failed testing.

attiks’s picture

Status: Needs work » Needs review
FileSize
4.88 KB

Next problem discovered, I have a label in English "car driver's name" with a translation to Japanese, but it didn't translate.

The problem is caused by the check $element['value']['#title'] == $instance['label'] because $element['value']['#title'] already run through check_plain changing the ' to '

attiks’s picture

Status: Needs review » Needs work

Same problem exists for file fields

attiks’s picture

Status: Needs work » Needs review
FileSize
5.45 KB

Support for file field added

attiks’s picture

FileSize
5.79 KB

another one, more check_plain added

pbuyle’s picture

Patch in #18 works, but for image fields the additional instructions (file size, extensions, image size) are lost.

attiks’s picture

For files the only thing that happens is setting the $element['#title'], so not sure what you mean by lost?
Are they there without the patch?

+++ b/i18n_field/i18n_field.moduleundefined
@@ -202,11 +201,26 @@ function i18n_field_field_widget_form_alter(&$element, &$form_state, $context) {
+    if ($field['type'] == 'file') {
+      $element['#title'] = $instance_current['label'];
pbuyle’s picture

Sorry, I meant an image field:
Image field additional instructions.

Here is a patch to keep the additional instructions by replacing the original description with the translated one in $element[0]['#description']

For a file field's label, $element['#title'] = $instance_current['label']; is not the only thing to happen. The following if() conditions are still checked and the following block could alter $element[0]['value']['#title'].

  if (!empty($element[0]['#title']) && $element[0]['#title'] == check_plain($instance['label'])) {
      $element[0]['#title'] = $instance_current['label'];
      if (isset($element[0]['value']['#title'])) {
        $element[0]['value']['#title'] = $element[0]['#title'];
      }

Note: There is something wrong with all these if() condition to alter the right $element child's properties. The key (either none, 0, 'value' or 'target_id' should be determined only once before altering the label and description.

attiks’s picture

Thanks for fixing, the code looks indeed a bit funny, but I'm launching in 48 hours, so I changed what was there.

martins.bertins’s picture

Status: Needs review » Needs work

Used the patch from #21.
It removed the label for boolean type field (list_boolean) when using the field label instead of on value.

martins.bertins’s picture

Status: Needs work » Needs review
FileSize
5.91 KB

Updated the patch with fix for list_boolean field.

watchdog’s picture

Nice! Patch in #24 works great so far, thanks

facine’s picture

#24 works!

Thanks

danieldumbrava’s picture

#24 works, but if you have any special characters in the title of the field, check plain will replace them, so the condition won't be fulfilled and any field that has special chars in the title won't be translated any more. Is check_plain actually needed in the if condition?

mradcliffe’s picture

Instead of only looking for an indexed form child, could the patch in #24 use element_children()? This is the best way of getting a form element's child keys. It also seems like #1964234: Not all fields are translated has some duplicate code from #24.

In my field module I have named keys for child elements.

Piazza’s picture

Patch in #24 works for me. Thanks!

benjifisher’s picture

To answer the question in #27: yes, check_plain() is needed. See #15 above for an explanation. If you find a case where there is a problem and removing the check_plain() fixes it, please say how to reproduce it.

czigor’s picture

#24 works for me too.

rutcreate’s picture

FileSize
6.19 KB

Updated the patch with fix for email field.

Status: Needs review » Needs work

The last submitted patch, i18n-field-label.patch, failed testing.

jibran’s picture

bojanz’s picture

Title: Label for Integer and File field types is not translated after upgrading to 7.x-1.8 » [7.x-1.8 regression] Field translation fails in numerous situations
Version: 7.x-1.8 » 7.x-1.x-dev
Priority: Major » Critical
Status: Needs work » Needs review
FileSize
8.21 KB

Well, this was not a fun day :)

Retitling the issue. Attaching a patch without #1961750: Unable to translate field label and description containing html tags/entities., it is a separate bug, so there's no need to fix it in this issue (since we're already fixing 5 different bugs here).

I've based my work on the #32 patch.
Tested text, numeric, image, entityreference, email, list text, list boolean fields of both cardinality 1 and unlimited.
Tested most widgets, tested default values.

Here are the different parts:

-  // Skip the node type edit fields by checking for existing entity
-  if (empty($element['#entity'])) {
+  // Don't translate if the widget is being shown on the field edit form.
+  if ($form_state['build_info']['form_id'] == 'field_ui_field_edit_form') {
     return;
   }

This is a much smarter check, allows us to be explicit about what we're doing (ignoring the Field UI edit form, though I have to ask: why?).

+  // Get the element to alter. Account for inconsistencies in how the element
+  // is built for different field types.
+  if (isset($element[0]) && count($element) == 1) {
+    // Single-value file fields and image fields.
+    $alter_element = &$element[0];
+  }
+  elseif (isset($element['value'])) {
+    // Number fields. Single-value text fields.
+    $alter_element = &$element['value'];
+  }
+  elseif ($field['type'] == 'entityreference' && isset($element['target_id'])) {
+    // Entityreference fields using the entityreference_autocomplete widget.
+    $alter_element = &$element['target_id'];
+  }
+  elseif ($field['type'] == 'email') {
+    // Email fields.
+    $alter_element = &$element['email'];
+  }
+  else {
+    // All other fields.
+    $alter_element = &$element;
+  }

It is very ugly to compare all versions of $element each time something inside needs to be changed.
Instead, I'm defining a new $alter_element before we start, and documenting what is used where.
Note that the first two IFs don't check for field types, because the same approach might make other field types work too.
The entityreference check is very specific, so that it fixes the autocomplete widget without breaking the rest.
I am not happy that we're hardcoding conditions for contrib modules, but that's what needs to be done.
A followup could perhaps introduce and document a hook that contribs could implement.

+    if (!empty($alter_element['#description'])) {
+      // Allow single-value file fields and image fields to have their
+      // descriptions translated. file_field_widget_form() passes the
+      // description through theme('file_upload_help'), so i18n_field
+      // must do the same.
+      $filefield = in_array($field['type'], array('file', 'image'));
+      $single_value = ($field['cardinality'] == 1);
+      $no_default = empty($alter_element['#default_value']['fid']);
+      if ($filefield && $single_value && $no_default) {
+        $help_variables = array(
+          'description' => $instance['description'],
+          'upload_validators' => $alter_element['#upload_validators'],
+        );
+        $original_description = theme('file_upload_help', $help_variables);
+        if ($alter_element['#description'] == $original_description) {
+          $help_variables = array(
+            'description' => $instance_current['description'],
+            'upload_validators' => $alter_element['#upload_validators'],
+          );
+          $alter_element['#description'] = theme('file_upload_help', $help_variables);
+        }
+      }

This new condition allows the description to be translated for single-value filefields and imagefields.

+  // Translate list options.
+  $has_options = (!empty($alter_element['#options']) || $field['type'] == 'list_boolean');
+  $has_allowed_values = !empty($field['settings']['allowed_values']);
+  $translate = i18n_field_type_info($field['type'], 'translate_options');
+  if ($has_options && $has_allowed_values && $translate) {
+    $alter_element['#options'] = $translate($field, $i18n_langcode);
+    if (isset($alter_element['#properties']) && !empty($alter_element['#properties']['empty_option'])) {
+      $label = theme('options_none', array('instance' => $instance, 'option' => $alter_element['#properties']['empty_option']));
+      $alter_element['#options'] = array('_none' => $label) + $alter_element['#options'];
+    }
+    // Translate list_boolean fields using the checkboxes widget.
+    if (!empty($alter_element['#title']) && $field['type'] == 'list_boolean' && !empty($alter_element['#on_value'])) {
+      $on_value = $alter_element['#on_value'];
+      $alter_element['#options'];
+      $alter_element['#title'] = $alter_element['#options'][$on_value];
     }
   }

This fixes list_boolean fields using both available widgets.

mradcliffe’s picture

Status: Needs review » Needs work

This is not a full review, but re-iterate my comment in #28.

+++ b/i18n_field/i18n_field.moduleundefined
@@ -176,8 +175,8 @@ function i18n_field_field_formatter_view($entity_type, $entity, $field, $instanc
@@ -189,8 +188,31 @@ function i18n_field_field_widget_form_alter(&$element, &$form_state, $context) {

@@ -189,8 +188,31 @@ function i18n_field_field_widget_form_alter(&$element, &$form_state, $context) {
   $instance = $context['instance'];
   $langcode = $context['langcode'];
 
+  // Get the element to alter. Account for inconsistencies in how the element
+  // is built for different field types.
+  if (isset($element[0]) && count($element) == 1) {
+    // Single-value file fields and image fields.
+    $alter_element = &$element[0];

Can you use element_children() here instead? An element child may be indexed or keyed, not just indexed.

Maybe something like this if you always want the first child element..?

$child_keys = element_children($element);

if (isset($child_keys[0])) {
  $alter_element = &$element[$child_keys[0]];
}
bojanz’s picture

Status: Needs work » Needs review

The point of that code is explicit, accounting for core modules (like file) which do:

$elements = array($element);
return $elements; 

Every IF clause there matches a specific set of field types.
I am wary of blindly modifying children because we don't know what results that will give for untested field types (for instance, you definitely don't want to do that if you have multiple numeric children, like when you have an unlimited cardinality field). So, we can only lose, not gain anything.
In general, trying to support any kind of arbitrary element structure with one alter hook is a really bad idea, but since we're forced to do it, I tried to do what was safest.

Razia_b’s picture

Thanks. Your patch has saved me a lot of hassle :)

mradcliffe’s picture

Okay, I guess i18n won't support all fields then. :|

Edit: I should really expand this instead of being disappointed.

I disagree completely. The correct way to find an element child is via element_children() whether it's an index or a key.

mErilainen’s picture

Not sure if this is the best approach, but it works like a charm, no more bi-lingual editor interface. All the translations I have added already now appear as they should after applying the patch.

jibran’s picture

Status: Needs review » Needs work

@bojanz great work in combining the patch and come up with one approach. I am still unable to translate date field description.

bojanz’s picture

I didn't test with date fields, so you'll probably need to provide a new patch that deals with date fields.
Supporting any type of field is impossible, we need to tackle field type by field type, unfortunately.

caktux’s picture

Boolean fields with the option 'Use field label instead of the "On value" as label' end up with no label at all.

bojanz’s picture

Updated patch.

Added this part:

  // If a subelement has the same title as the parent, translate it instead.
  // Allows fields such as email and commerce_price to be translated.
  foreach (element_get_visible_children($element) as $key) {
    $single_value = ($field['cardinality'] == 1);
    $has_title = isset($element['#title']) && isset($element[$key]['#title']);
    if ($single_value && $has_title && $element[$key]['#title'] == $element['#title']) {
      $alter_element = &$element[$key];
      break;
    }
  }

allows email fields to work without hardcoding them in the conditions above, and allows commerce_price fields to work as a bonus.

I have also confirmed that the set of IFs can be replaced with this code:

  // Get the element to alter. Account for inconsistencies in how the element
  // is built for different field types.
  // Single-value file fields and image fields are under $element[0].
  // Number fields and single-value text-fields are under $element['value'].
  // Entityreference fields using the entityreference_autocomplete widget
  // are under $element['target_id'].
  if (!isset($element['#title'])) {
    foreach (element_get_visible_children($element) as $key) {
      if (isset($element[$key]['#title'])) {
        $alter_element = &$element[$key];
        break;
      }
    }
  }

which is the approach mradcliffe has advocated, but I'm still unsure about the magic here, especially relying on an empty #title to trigger this check.
That's why I haven't added it to the patch (yet)

Jose Reyero’s picture

Status: Needs work » Fixed

After reading through all this and weighting pros and cons I've committed the patch in #35. Thanks everybody here because translating fields is a really difficult one. Some notes:
- This fields handling code is never going to look nice, se the best we can do is commenting all the if/then mess. Thanks @bojanz for extensive documentation in the patch.
- For this module we really have enough with translating all Drupal core field types -and not breaking contrib forms-. For translating fields provided by contrib modules, like Date and Entity references, please ,https://drupal.org/project/i18n_contrib
- Replying to some question above, we are not even trying to translate field config forms becase 1. 'field' names afaik do not show up for end user forms -as oppsed to 'field instance' ones- and 2. we have already a lot of work with trying to translate end user forms.
- Right about filtering issues, that's some other issue, please give a try to #1961750: Unable to translate field label and description containing html tags/entities.
- Yes, I know regressions are specially ugly... More tests are always welcome.
- Sorry if I forgot some names in the commit message, I've just used the one generated by Dreditor.

Btw, aiming at a new module release within the next two weeks, please do some testing.

bojanz’s picture

Thanks! Do you have any feedback regarding the changes in #44? The first part (that's included in the patch) could especially be useful, otherwise I need to figure out how to add support for commerce_price fields (some kind of a hook? or hardcoding it too?).

jibran’s picture

@bojanz please create a follow up patch/issue. Meanwhile I have posted new patch at #1961750-1: Unable to translate field label and description containing html tags/entities..
Thanks @Jose Reyero for committing the patch.

Status: Fixed » Closed (fixed)

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

hoporr’s picture

Used patch from #35, which was committed in #45.

Seeing issues with Boolean fields: labels are not shown any longer, instead only seeing Yes|No.
May be related to #23, #24?

bojanz’s picture

Please open a followup issue for that.

hoporr’s picture

Followup issue for #49, Boolean fields problem: https://drupal.org/node/2029043
(already existed)

Jose Reyero’s picture

I think we should fix all these field issues for good. And this is the way:

#2061999: Add tests for field translations to avoid regressions (i18n_field.test)

junetellain626’s picture

Issue summary: View changes

For those who might also have a hard time with getting their reference autocomplete widgets to work even with the #35 patch already committed. Double check if you the field you are using is indeed from the entityreference module, or from a node/user reference from the references module. [facepalm]
Adding the following to line 205 would be a temporary fix for node references field.

elseif ($field['type'] == 'node_reference' && isset($element['nid'])) {
    // Node reference fields using the node_reference_autocomplete widget.
    $alter_element = &$element['nid'];
  }
Pere Orga’s picture

Status: Closed (fixed) » Active

Running 1.10 I'm not able to translate the 'Email' field label of accounts, in the user view page (.e.g. user/1).

Sorry to reopen but, does anyone remember testing this while fixing the other issues of this ticket? Has anyone experienced this issue?

Jose Reyero’s picture

Status: Active » Closed (fixed)

@Pere,
That should be a regular text, translated through localization, nothing to do with this one.

And please don't reopen issues unless you read carefuly the previous 50 posts... (just joking, please don't)

Carlos Miranda Levy’s picture

This situation remains as of i18n 7.x-1.11.
At least for Entity Reference fields referencing User Profiles.

Not sure if this is duplicate of https://www.drupal.org/node/1961750

The last submitted patch, 35: 1904368-fix-i18n-field-34.patch, failed testing.

idebr’s picture

This page is ranked highly in Google for untranslatable field labels/descriptions with i18n.

Future visitors: this is the issue you are looking for: #2474403: Translation of field description overwritten