Hi,

I noticed that some list fields which use the allowed values function option are not correctly translated. Instead, it shows the (untranslated) keys, rather than the values. I think it boils down to an empty option list being returned in the following function:

function i18n_field_translate_allowed_values($field, $langcode = NULL) {
  if (!empty($field['settings']['allowed_values'])) {
    return i18n_string_translate(array('field', $field['field_name'], '#allowed_values'), $field['settings']['allowed_values'], array('langcode' => $langcode));
  }
  else {
    return array();
  }
}

If I use the following piece of code instead, it seems to work fine, but I am not sure if this is correct:

function i18n_field_translate_allowed_values($field, $langcode = NULL) {
  if (!empty($field['settings']['allowed_values'])) {
    return i18n_string_translate(array('field', $field['field_name'], '#allowed_values'), $field['settings']['allowed_values'], array('langcode' => $langcode));
  }
  else if (!empty($field['settings']['allowed_values_function'])) {
    return call_user_func($field['settings']['allowed_values_function']);
  }
  else {
    return array();
  }
}

Please note that the callback I use for the options returns untranslated keys, but translated labels, i.e.:

function options() {
  return array(
    'key' => t('Label'),
  );
}
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jose Reyero’s picture

Status: Active » Postponed (maintainer needs more info)
Jose Reyero’s picture

Status: Postponed (maintainer needs more info) » Closed (cannot reproduce)

No follow up, so closing.

Jelle_S’s picture

Version: 7.x-1.7 » 7.x-1.x-dev
Status: Closed (cannot reproduce) » Active

Issue is still valid.

If you create a rule with a "data comparison" condition and you select a field that has an 'allowed_values_function' setting, the UI displays an empty select box to choose from. I've boiled it down to this particular function. Applying the change from the issue summary works. Patch will follow.

Jelle_S’s picture

Status: Active » Needs review
FileSize
827 bytes

Status: Needs review » Needs work

The last submitted patch, 4: i18n-allowed-values-function-1829050-4.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 4: i18n-allowed-values-function-1829050-4.patch, failed testing.

Jelle_S’s picture

Status: Needs work » Reviewed & tested by the community

There's something weird going on with the bot:

Fatal error: Cannot redeclare system_requirements() (previously declared in /var/lib/drupaltestbot/sites/default/files/checkout/modules/system/system.install:11) in /var/lib/drupaltestbot/sites/default/files/checkout/sites/default/modules/rebuild/tests/fixtures/drupal_sites/dev/modules/system/system.install on line 518
Drush command terminated abnormally due to an unrecoverable error.       [error]

Patch works on my local machine.

Jose Reyero’s picture

Category: Bug report » Feature request
Status: Reviewed & tested by the community » Needs work

Not without green tests.

Also this is a callback funcion, not sure we really want to translate those.

fengtan’s picture

Status: Needs work » Needs review
FileSize
954 bytes

We also have issues with this function: we have a field with allowed values defined in a allowed_values_function callback, and we expose it in a facet. The problem is that the machine names (instead of the labels) show up in the facet.
Here is a tentative patch -- it fixes the problem for us. Tests are green locally.

lambic’s picture

Status: Needs review » Reviewed & tested by the community

patch looks good

delmarr’s picture

Status: Reviewed & tested by the community » Needs work

Doesn't work for me. When I apply Fengtan patch the machine name still is displayed in the facets block after re-indexing

youngelpaso’s picture

Status: Needs work » Reviewed & tested by the community

From our own local tests, the issues reported by Delmarr are similar, but unrelated. The patch above works according to tests.

Jose Reyero’s picture

Status: Reviewed & tested by the community » Needs work

As I understand the issue this is not about "allowed values being translated", but about "wrong allowed values" (untranslated keys are shown).
On the other side we shouldn't be translating 'allowed values' returned by any other callback function, because those may be already translated, or not, we just don't know.

So I think the kind of logic we need here is:
- Translate allowed values if there are some in field settings.
- Keep hands of if there's a callback function to be invoked.

We also need to know more about the fields causing issues, because it seems some other module is adding an 'allowed values function' to some core fields that didn't have any... Could you please provide a field info dump of the fields causing issues?

Göran’s picture

Error for me from Git apply, and if I manually try this changes directly in the code I will get error when running.

fengtan’s picture

FileSize
2.05 KB

@Jose Attached is the field info dump of the field. I also have this in mymodule.module:

function mymodule_mycallback() {
  return array(
    'key1' => 'Value 1',
    'key2' => 'Value 2',
    'key3' => 'Value 3',
  );
}

If I run print_r(i18n_field_translate_allowed_values(field_info_field('field_myfield'))) then:
* Without the patch, I get

Array()

* With the patch, I get

Array
(
    [key1] => Value 1
    [key2] => Value 2
    [key3] => Value 3
)

I believe the latter is the expected behaviour.

The allowed_values_function callback is not much documented but is provided by list.module (see code of list_allowed_values()).
Some unofficial documentation:
* http://www.phase2technology.com/blog/setting-the-allowed-values-function...
* http://dropbucket.org/node/2008
* There is also an example in the core simpletests (see source of ListDynamicValuesTestCase)

In our case the facet displays machine names as a fallback because i18n_field_translate_allowed_values() returns an empty array and thus does not allow the facet to map machine names with human names.

Afaik the allowed_values_function callback is supposed to return untranslated values.

@Göran the patch seems to apply happily when using simplytest:
Did you apply it on 7.x-1.x-dev ? What error message do you have ?

Imho patch #10 is still valid. Thoughts ?

fengtan’s picture

Status: Needs work » Needs review
gapple’s picture

I ran into this problem using Entity API, where $node_wrapper->field_select_list->label() returned empty values.
The callstack is:

  EntityMetadataWrapper::label()
  EntityMetadataWrapper::optionsList()
  entity_metadata_field_options_list()
  i18n_field_translate_allowed_values()

--

Documentation of allowed_values_callback in D7 is in hook_options_list, where the examples are passed through t()

In Drupal 8, Documentation is callback_allowed_values_function, where the values are untranslated.

However, I don't think that will affect this issue, since translations are loaded by i18n context (e.g. "field_select_list:#allowed_values:item_key"). Either the t() values are passed through unmodified for the default language, or are overwritten with the i18n translation based on the option keys.

ciss’s picture

Status: Needs review » Needs work

We use option list callbacks quite a lot (in combination with https://www.drupal.org/project/list_predefined_options) to ease development and updates.

I have to agree with #14. While I can see valid arguments both for translating and not translating returned labels, I would argue that it might be best to err on the side of caution and only ensure that a callback's return value gets passed through, at least until all implications of passing those labels through i18n have been properly evaluated.

One can always alter a list options callback in order to add middleman translation. Doing the opposite (circumventing i18n for special cases) is far more difficult.

ciss’s picture

A quick follow-up: Circumventing i18n is actually easier then I expected, as it only requires hook_i18n_field_info_alter() to monkey-patch $info['list_text']['translate_options']. But since basically all examples for the allowed_values_function use t() for their labels we have to expect that this will be the common case.

In addition the patch changes the current handling of empty lists. This is an unnecessary change that may cause problems down the line (e.g. in custom textgroups).

ciss’s picture

Status: Needs work » Needs review
FileSize
1.11 KB
jrochate’s picture

hi.

This patch solved the problem I was having under Commerce Shipping (when using Calculation Rules).

I've tested other sections on my site, and patch didn't break it, so it could be commited.

ciss’s picture

Status: Needs review » Reviewed & tested by the community

Patch in #21 still applies. Setting to RTBC per comment #22.

ricovandevin’s picture

Patch in #21 applies cleanly against 7.x-1.26 and fixes the issue. It also is the right approach IMHO. Please commit.

  • joseph.olstad committed 3b532df on 7.x-1.x authored by ciss
    Issue #1829050 by fengtan, ciss, Jelle_S: Translate field options with...
joseph.olstad’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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