We had the same problem but the pmkanse fix works in latest version: 9.5.7
- last update
about 1 year ago 30,334 pass - ๐ฏ๐ดJordan Rajab Natshah Jordan
Facing this issue in Drupal 10.1.0-beta1
- ๐ฎ๐ณIndia lolmanKD
We are facing this issue on Drupal 9.2.20, I didn't saw the version support for it so created a new for it.
- last update
about 1 year ago 29,417 pass 22:25 18:01 Running- last update
about 1 year ago 29,415 pass - Status changed to Needs review
11 months ago 8:42pm 22 July 2023 - last update
11 months ago 29,842 pass, 11 fail - ๐ณ๐ฑNetherlands Lendude Amsterdam
Came here from ๐ WSOD on admin/modules if description is set but is NULL in module.info.yml Fixed , where this was raised as a possible fix, but this seems unrelated.
Here is a test for this.
This problem arises when a config item is marked as translatable or uses a translatable type such as 'label', but contains something other than a string. #28 is a good example of this where schema says its a sting and the config is an array.
-
+++ b/core/lib/Drupal/Core/StringTranslation/TranslatableMarkup.php @@ -127,6 +127,7 @@ class TranslatableMarkup extends FormattableMarkup { public function __construct($string, array $arguments = [], array $options = [], TranslationInterface $string_translation = NULL) { + $string = !empty($string) ? $string : '';
We should not be changing TranslatableMarkup
-
+++ b/core/modules/locale/src/LocaleConfigManager.php @@ -175,7 +175,7 @@ protected function getTranslatableData(TypedDataInterface $element) { - if (!empty($definition['translatable']) && $value !== '' && $value !== NULL) { + if (!empty($definition['translatable']) && (($value !== '' && $value !== NULL) && is_string($value))) {
Do we want to be fault tolerant here and just skip this? Or should we throw something to indicate something it trying to translate something untranslatable? The currently suggested fix would leave users without any indication that anything is wrong. But it doesn't necessarily feel like locales responsibility to inform about faulty config
-
- last update
11 months ago 29,880 pass, 1 fail - ๐ณ๐ฑNetherlands Lendude Amsterdam
This implements the previous proposed fix of skipping the faulty config, but somewhat refactored.
The last submitted patch, 56: 2719553-56-TEST_ONLY.patch, failed testing. View results โ
The last submitted patch, 57: 2719553-57.patch, failed testing. View results โ
- Status changed to Needs work
11 months ago 9:51pm 22 July 2023 - ๐ช๐ธSpain penyaskito Seville ๐, Spain ๐ช๐ธ, UTC+2 ๐ช๐บ
+++ b/core/modules/locale/src/LocaleConfigManager.php @@ -175,7 +175,7 @@ protected function getTranslatableData(TypedDataInterface $element) { + if (!empty($definition['translatable']) && $value !== '' && is_string($value)) {
As there are some discussions ( #2275865: Per language settings (vs translated settings) are not directly supported โ ) around how to localize configuration (even when they are not strings) and this is subject to change, I would use !is_array instead of asserting for is_string().
Even if we never promised to support that you can translate non-labels, I'm pretty sure people is doing that in custom or even contrib modules. This could break their assumptions, which might be ok-ish but we should avoid if we can.
- Status changed to Needs review
11 months ago 6:43am 23 July 2023 - last update
11 months ago 29,881 pass - ๐ณ๐ฑNetherlands Lendude Amsterdam
Fixing the failing test.
@penyaskito thanks for linking that, hadn't seen that issue yet. In this case though, the current way getTranslatableData works will always send the value to TranslatableMarkup and since that does this:
if (!is_string($string)) { $message = $string instanceof TranslatableMarkup ? '$string ("' . $string->getUntranslatedString() . '") must be a string.' : '$string ("' . (string) $string . '") must be a string.'; throw new \InvalidArgumentException($message); }
anything that can't get passed is_string() will cause an exception. So I think keeping the check in getTranslatableData the same as in TranslatableMarkup would be the best way to prevent the exception. Should we change in the future what we allow to be translated, we probably need new logic here anyway.
But again, I'm not 100% convinced that preventing the exception is the right thing to do here. It's just that the current exception makes it very unclear what is going wrong. I'd be totally on board with catching the current exception and re-throwing it with a clearer message that a specified config setting can't be translated or something along those lines.
- ๐ณ๐ฑNetherlands Lendude Amsterdam
@penyaskito So yeah, maybe we shouldn't use is_string, but catch the exception? That way the logic of what can be translated stays within TranslatableMarkup and any changes would work in getTranslatableData too....
Sill leaves the question about what we want to do if we catch that exception, silently move along or log something or rethrow it?
- Status changed to Needs work
11 months ago 6:10pm 25 July 2023 - ๐บ๐ธUnited States smustgrave
@Lendude think something should be logged and move along.
Lot of my sites I routinely check the logs for issues and know that's when I would fix that.
- Status changed to Needs review
11 months ago 9:42am 27 July 2023 - last update
11 months ago 29,882 pass, 2 fail - ๐ณ๐ฑNetherlands Lendude Amsterdam
Ok lets log something!
This requires the logger factory to be added, so added a deprecation and change record for that.
Added test coverage for the error.Not sure about the wording of the error, best I could come up with.
The last submitted patch, 64: 2719553-64.patch, failed testing. View results โ
- last update
11 months ago 29,887 pass - Status changed to RTBC
11 months ago 4:23pm 30 July 2023 - last update
11 months ago 29,910 pass - ๐ช๐ธSpain penyaskito Seville ๐, Spain ๐ช๐ธ, UTC+2 ๐ช๐บ
I think this is the best we can do for having _some_ error handling while still not breaking what people might be doing as in #60. Thanks!
Might be good having a summary update, or at least a title update. I tried to do that, but suggestions welcome.
- last update
11 months ago 29,948 pass - last update
11 months ago 29,948 pass - last update
11 months ago 29,955 pass - last update
11 months ago 29,955 pass - last update
10 months ago 29,955 pass, 1 fail - Status changed to Needs work
10 months ago 10:14pm 9 August 2023 The last submitted patch, 66: 2719553-66.patch, failed testing. View results โ