- 🇳🇿New Zealand quietone
The change record here also needs to be reviewed and the branch/version updated.
- last update
over 1 year ago Patch Failed to Apply
While trying to write a multilingual test for #2449597: Number formatters: Make it possible to configure format_plural on the formatter level → , I noticed the following problem.
The prefix and suffix settings on number fields have these descriptions:
Define a string that should be prefixed to the value, like '$ ' or '€ '. Leave blank for none. Separate singular and plural values with a pipe ('pound|pounds').
Define a string that should be suffixed to the value, like ' m', ' kb/s'. Leave blank for none. Separate singular and plural values with a pipe ('pound|pounds').
So it's asking you to use | to separate your singular and plural values. This in itself has problems, because only some languages have singular/plural values -- in many languages there is either only 1 form or there are more than 2, and in some although there are two forms, they don't correspond to "singular" and "plural".
So that is one problem.
Then, in \Drupal\Core\Field\Plugin\Field\FieldFormatter\NumericFormatterBase::viewElements(), there is a blatant misuse of formatPlural() and translations in general for these fields
$prefixes = isset($settings['prefix']) ? array_map(array($this, 'fieldFilterXss'), explode('|', $settings['prefix'])) : array('');
$suffixes = isset($settings['suffix']) ? array_map(array($this, 'fieldFilterXss'), explode('|', $settings['suffix'])) : array('');
$prefix = (count($prefixes) > 1) ? $this->formatPlural($item->value, $prefixes[0], $prefixes[1]) : $prefixes[0];
$suffix = (count($suffixes) > 1) ? $this->formatPlural($item->value, $suffixes[0], $suffixes[1]) : $suffixes[0];
The problems here:
a) formatPlural assumes that it is getting English text. Here it is actually getting the already-translated text from the config object (translated into the interface language, or the entity language, or whatever). So calling formatPlural() on that will be adding some non-English text into the translation database as English sources. And trying to translate this text, which is already translated, is incorrect/pointless anyway.
b) formatPlural assumes it can translate the English input by pasting it together with a special string that is NOT |, and look up the translation to the target language, which might have n != 2 plural forms. Here, it is only getting two forms from NumericFormatterBase::viewElements() (no matter what language it is or how many forms someone might have entered, separated by |, in the config). Plus, the config string that is the source has | as the separator, not the usual character.
So even if it was always passed in as English (which it isn't), the source string would be something like "Singular|Plural" in the translation database (that is what is stored in the English source config), but formatPlural would be trying to look up something like "Singular[CHAR]Plural", where [CHAR] is the special plural form separator that formatPlural uses, in its database, and it would never find the translation of the configuration.
c) Once those plural forms are obtained from translation, formatPlural will attempt to use the plural rules for the target language to make the correct string. But this will not work here, even if you pass in the translated forms, because for instance, in Russian you need 3 forms to do that and you'll only have passed in 2.
So this is just plain wrong. It may work for English-only sites, but it doesn't work for sites whose main language has != 2 plurals, and definitely will not work with translation at all.
Drupal 7 had prefix/suffix capability on numeric fields, so we do not want to lose the capability of having prefixes and suffixes on numeric fields. Otherwise, we could just remove the prefix/suffix fields that do not work and be done.
Also, Drupal 7 had the ability for numeric fields in Views to have plural formatting, which we lost for entity fields when we did #2342045: Standard views base fields need to use same rendering as Field UI fields, for formatting, access checking, and translation consistency → . So we had issue #2449597: Number formatters: Make it possible to configure format_plural on the formatter level → , but then we decided on that issue that we would mark it as a duplicate of this issue, because it doesn't really make sense to have both prefix/suffix and the format_plural stuff; they pretty much do the same thing. Also we would need to have update/migrate functions that take old prefix/suffix things and migrate/update them to the new format_plural things. So the issues were better combined into one.
So besides the bug described above, this issue is also about restoring some functionality that we had in Drupal 7 and early versions of Drupal 8, which allowed numeric base fields on entities to be formatted with plural formatting in views.
a) Take the current, separate prefix/suffix settings out of the Field settings. Replace it with a single setting that supports formatPlural() properly. A user would have N fields to enter information in (for a language with N plural variants), and in the UI they would enter things like:
$@count
1 fish
@count fish
etc.
The field settings code would take the UI and paste it together with the correct formatPlural sep character to save in the settings, and extract the variants into the UI to display to the user.
Here are before/after screen shots of the field settings... Before this patch:
With the patch:
We'll also need to update the previous prefix/suffix settings on the field to this new format-plural setting, using a hook_update_N() function.
b) The widget for numeric fields would have a checkbox to display the new format-plural setting as a prefix and suffix. Similar to what it is doing now with prefix/suffix, it would choose the variant for the last plural setting. Note that the widget currently (without the patch) does not have an option for this -- it is always printing out the prefix/suffix no matter waht -- so for update purposes this setting should default to TRUE.
Screen shot of this setting, with the patch (without this patch there is not a setting for this):
And here is what the editing screen looks like, with this patch or without it (that doesn't change, assuming the option is checked with the patch) -- it displays the plural form prefix/suffix before/after the field:
c) The formatter for numeric fields would have a checkbox to choose whether to:
1. Use the format-plural setting from the field
2. Use its own format-plural setting (similar UI to Field setting)
3. Not use format-plural
If 1 or 2 is chosen, it would use formatPluralTranslated to do it right, since the config would already be translated.
Here is what the settings look like before the patch:
And with the patch:
And here is what the formatted field output looks like (with or without the patch; this doesn't change):
Updating: The formatter currently just has a checkbox of whether to use or not use the prefix/suffix, so that would need to be migrated into "Use the format-plural setting from the field" or "Not use format-plural".
d) We'll need tests for the field settings, widget, formatter, and the update, and we'll need to have the patch update stored Views and View Mode and Form Mode configs, although there may not be many that are affected.
e) We'll also need to update the migration from D6/7 so that it works with the new settings.
1. Make a patch. [done -- see screenshots above]
Patch Credit: This patch incorporates most of the patch from
#2449597: Number formatters: Make it possible to configure format_plural on the formatter level →
so we should credit the people who put in work on that patch here as well. These include:
mpdonadio, Gábor Hojtsy, yched, jhodgdon, pcambra, effulgentsia, rteijeiro, dawehner
If they comment here, they should be credited here.
2. Write a change notice. [done]
3. Review/commit.
Non-workable prefix/suffix pluralization stuff will be removed and replaced with working pluralization for field formatters and widgets, restoring functionality that was present in Drupal 6.
No. Some additions.
The configuration schema for numeric fields, widgets, and formatters changes (with an update function). The old schema model was not actually workable, as described above.
See also Beta table above, which explains the disruption/status/etc. issues.
I think that this should go in sooner rather than later, to minimize disruption. It changes the config schema for numeric fields, formatters, and widgets. There is an update function provided, which will take care of active config, but that doesn't automatically take care of config in config/install and config/optional directories. To minimize the impact on contrib, it seems like stabilizing this configuration earlier is better.
There are not API changes, so other than exported config files, there shouldn't be disruption.
Needs work
11.0 🔥
(Drupal 8 Multilingual Initiative) is the tag used by the multilingual initiative to mark core issues (and some contributed module issues). For versions other than Drupal 8, use the i18n (Internationalization) tag on issues which involve or affect multilingual / multinational support. That is preferred over Translation.
Not all content is available!
It's likely this issue predates Contrib.social: some issue and comment data are missing.
The change record here also needs to be reviewed and the branch/version updated.