Why this should be an RC target
This is a subtle bug that will result in some strings failing to be translated properly, see e.g. #7. Contrib may end up in adding more failing calls. The proposed fix in #14 addresses the issue at the root API level by enforcing proper defaults in case NULL arguments are passed in. No disruption AFAICS.
- Follow-up to
🐛
GD toolkit & operations should catch \Throwable to fail gracefully in case of errors
Fixed
- Former title: format_size() erroneously passes NULL as langcode in options array
Problem
File common.inc, function format_size()
This function either calls:
\Drupal::translation()->formatPlural($size, '1 byte', '@count bytes', array(), array('langcode' => $langcode));
or:
$options = ['langcode' => $langcode];
...
return new TranslatableMarkup('@size ...', $args, $options);
However, the parameter $langcode may be null or absent with a default of null. in which case the $option array contains an entry 'langcode' with a value of null.
Deeper down this leads to:
- Missing merging in defaults (core\lib\Drupal\Core\StringTranslation\TranslationManager.php:doTranslate():
// Merge in options defaults.
$options = $options + [
'langcode' => $this->defaultLangcode,
'context' => '',
];
- Checking/assigning key NULL (core\lib\Drupal\Core\StringTranslation\Translator\StaticTranslation.php:getStringTranslation()):
if (!isset($this->translations[$langcode])) {
$this->translations[$langcode] = $this->getLanguage($langcode);
}
- Checking/assigning key NULL; implicitly converting NULL to string; Missing cache (or even using cache built in another language?) via core\modules\locale\src\LocaleTranslation.php:getStringTranslation() and core\modules\locale\src\LocaleLookup.php:getCid():
if (!isset($this->translations[$langcode][$context])) {
$this->translations[$langcode][$context] = new LocaleLookup($langcode, $context, $this->storage, $this->cache, $this->lock, $this->configFactory, $this->languageManager, $this->requestStack);
}
$this->cid = "locale:{$this->langcode}:{$this->context}:$rids";
From these examples it may be clear that in Drupal translation:
- A parameter $langcode may be left out or null if explicitly indicated so.
- A parameter $langcode may not be null (or the empty string ) if it is documented as being of type string.
- A key 'langcode' in a $options parameter of type array may be absent but not null.
Errors against this pattern are also found in:
- \Drupal\Core\Datetime\DateFormatter::formatInterval()
- \Drupal\Core\Datetime\DrupalDateTime::format(), formatDiff()
- File system.tokens.inc, function system_tokens(), call to formatTimeDiffSince
- \Drupal\Core\Validation\DrupalTranslator
Proposed resolution
Let the translation system accept null or the empty string as value for the key langcode in a $options array parameter or as $langcode parameter.
Note: why also the empty string? Because NULL converted to a string results in the empty string. Config/ini/key-value store components often only return strings using the empty string for an absent value. Example: \Drupal\Core\StringTranslation\TranslatableMarkup::getOption() returns the empty string if an option is set to null.
Remaining tasks
- Review patch.
- Accept patch.
User interface changes
None.
API changes
None, as values accepted were not strictly defined/documented. After the patch, more values are accepted and handled as expected.
Data model changes
None.