Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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'),
);
}
Comment | File | Size | Author |
---|---|---|---|
#21 | i18n-allowed-values-function-1829050-21.patch | 1.11 KB | ciss |
#16 | field_info_myfield.txt | 2.05 KB | fengtan |
#10 | i18n-allowed-values-function-1829050-10.patch | 954 bytes | fengtan |
#4 | i18n-allowed-values-function-1829050-4.patch | 827 bytes | Jelle_S |
Comments
Comment #1
Jose Reyero CreditAttribution: Jose Reyero commentedThis may be fixed by #1875282: Improve field translation by implementing hook_field_widget_form_alter()
Please give it a try
Comment #2
Jose Reyero CreditAttribution: Jose Reyero commentedNo follow up, so closing.
Comment #3
Jelle_SIssue 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.
Comment #4
Jelle_SComment #8
Jelle_SThere's something weird going on with the bot:
Patch works on my local machine.
Comment #9
Jose Reyero CreditAttribution: Jose Reyero commentedNot without green tests.
Also this is a callback funcion, not sure we really want to translate those.
Comment #10
fengtanWe 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.
Comment #11
lambic CreditAttribution: lambic commentedpatch looks good
Comment #12
delmarr CreditAttribution: delmarr commentedDoesn't work for me. When I apply Fengtan patch the machine name still is displayed in the facets block after re-indexing
Comment #13
youngelpaso CreditAttribution: youngelpaso commentedFrom our own local tests, the issues reported by Delmarr are similar, but unrelated. The patch above works according to tests.
Comment #14
Jose Reyero CreditAttribution: Jose Reyero commentedAs 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?
Comment #15
Göran CreditAttribution: Göran commentedError for me from Git apply, and if I manually try this changes directly in the code I will get error when running.
Comment #16
fengtan@Jose Attached is the field info dump of the field. I also have this in
mymodule.module
:If I run
print_r(i18n_field_translate_allowed_values(field_info_field('field_myfield')))
then:* Without the patch, I get
* With the patch, I get
I believe the latter is the expected behaviour.
The
allowed_values_function
callback is not much documented but is provided bylist.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 ?
Comment #17
fengtanComment #18
gappleI ran into this problem using Entity API, where
$node_wrapper->field_select_list->label()
returned empty values.The callstack is:
--
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.Comment #19
ciss CreditAttribution: ciss at yousign GmbH commentedWe 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.
Comment #20
ciss CreditAttribution: ciss at yousign GmbH commentedA 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).
Comment #21
ciss CreditAttribution: ciss at yousign GmbH commentedComment #22
jrochate CreditAttribution: jrochate at ADJ 3 Sistemas commentedhi.
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.
Comment #23
ciss CreditAttribution: ciss at yousign GmbH commentedPatch in #21 still applies. Setting to RTBC per comment #22.
Comment #24
ricovandevin CreditAttribution: ricovandevin at Finlet for One Shoe commentedPatch in #21 applies cleanly against 7.x-1.26 and fixes the issue. It also is the right approach IMHO. Please commit.
Comment #26
joseph.olstad