Problem/Motivation
- Enable any language ('de' for example);
- Enable 'content_moderation' module;
- Update translations
- Export configuration
- Import exported configuration on a clean site
I get following error for translation of config ' workflows.workflow.editorial.yml':
TypeError: Argument 5 passed to Drupal\locale\LocaleConfigSubscriber::processTranslatableData() must be of the type array, boolean given, called in /var/www/drupalvm/sites/valeod8/docroot/core/modules/locale/src/LocaleConfigSubscriber.php on line 152 in /var/www/drupalvm/sites/valeod8/docroot/core/modules/locale/src/LocaleConfigSubscriber.php on line 142 #0 /var/www/drupalvm/sites/valeod8/docroot/core/modules/locale/src/LocaleConfigSubscriber.php(152): Drupal\locale\LocaleConfigSubscriber->processTranslatableData('workflows.workf...', Array, Array, 'de', false)
Proposed resolution
This looks to be a bug in LocaleConfigSubscriber::processTranslatableData
.
While iterating over the localized config data, it keeps a reference to the original configuration so that the localized configuration serves as an override. However, it was replacing it's reference to $reference_config
causing it to reference the wrong value when iterating over multiple keys.
Renaming the variable to avoid overriding the original should fix the problem.
Remaining tasks
Write patchWrite a test- Review
User interface changes
N/A
API changes
None
Data model changes
None
Release notes snippet
TBD?
Comment | File | Size | Author |
---|---|---|---|
#26 | 2939356-25.patch | 974 bytes | rzanetti |
Comments
Comment #2
angel.h@validoll did you find a solution for this?
Comment #3
validoll@angel.h unfortunately no. Just remove translations of this config.
Comment #4
justclint CreditAttribution: justclint as a volunteer commentedAlso ran into this. Removing workflows.workflow.editorial.yml from each translation resolved the error for now.
Comment #5
mediabounds CreditAttribution: mediabounds commentedI ran into this issue too.
This looks to be a bug in LocaleConfigSubscriber.php in processTranslatableData.
While iterating over the localized config data, it keeps a reference to the original configuration so that the localized configuration serves as an override. However, it was replacing it's reference to
$reference_config
causing it to reference the wrong value when iterating over multiple keys.I'm attaching a patch to keep the local reference separate from the original argument.
This bug likely had other manifestations, but workflows configuration was somewhat of a special case (which is what caused the exception):
The default workflows has a "published" state and each state has a "published" property. Because processTranslableData was replacing it's reference to the original configuration, it ended up referencing the wrong "published" property in the config (in this case, a boolean instead of an array, hence the exception).
Comment #6
justclint CreditAttribution: justclint as a volunteer commentedI just tested #5 and it worked great. I can confirm the build including translation workflows.workflow.editorial.yml files completed without the errors now.
Thanks for the patch @mediabounds!
Comment #7
Rob230 CreditAttribution: Rob230 as a volunteer commentedPatch #5 fixes it for me, thanks.
Comment #8
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation for Pfizer, Inc. commentedNice work hunting this down @mediabounds!
My guess is this will need tests in order to get in, tagging.
Comment #9
Sharique CreditAttribution: Sharique at Acquia commented+1 for RTBC, the patch fixes the issue for me.
Comment #10
hctomPatch from #5 works like a charm, also +1 for RTBC... and do we really need a tests for fixing a new variable accidentally overriding an existing one inside a function?
Comment #11
hctomPerhaps I have to revoke my last comment: I applied the patch and from now on, our exported language entities loose their third-party settings (we have disable_language installed and those settings are lost upon export). First config export after fresh install and config import does not export any changes, but as soon as any config in the system is changed and exported, the problem with the lost third-party settings occures :(Perhaps someone else can also try it with the patch applied, disable_language enabled, some languages configured (with some disabled), so we can be sure if this is a problem with the patch or something else.Changing my mind back to comment #10 ;) We tracked down the issue with the lost third party settings and it turned out to be a
config_filter
misconfiguration for the language entity configs.So from my point of view: good to get it in as soon as possible.
Comment #12
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation for Pfizer, Inc. commentedThanks for reporting and the information @hctom @Sharique
After thinking about it, I agree this may not need a new test, as it could indeed have other scenarios where this would be affecting and it would be hard to track all down. I'm marking this as RTBC to get feedback from committers around this, other than this, this is a very straight fix and an easy to follow change.
Comment #13
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation for Pfizer, Inc. commentedUpdating the issue summary for clarity and current state of the issue.
Comment #14
alexpottThanks for finding and working out how to fix this bug. The fix looks great but it definitely needs a test. Tests prevent making the same mistake and this goes to show that our current test coverage of \Drupal\locale\LocaleConfigSubscriber::processTranslatableData() is not as complete as it should be.
Comment #15
LendudeHere is a test for this. It's a little forced to get the reported error to show.
Test only patch is the interdiff.
Comment #17
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation for Pfizer, Inc. commentedThanks @Lendude, - just updating the remaining tasks here :)
Comment #18
LendudeQuick clean up
Comment #19
LendudeInterdiff...
Comment #20
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation for Pfizer, Inc. commentedI had a look at the test, it makes sense to me, and the test only patch fails with the reported error demonstrating the bug.
Comment #21
alexpottCrediting @Manuel Garcia for issue management and review
Crediting @validoll for filing the issue.
Crediting @justclint and @hctom for testing the patch and reporting back
Committed and pushed ea0ab7e57a to 8.8.x and 4de8670ac1 to 8.7.x. Thanks!
Comment #24
vijaycs85Comment #26
rzanetti CreditAttribution: rzanetti at CI&T for Johnson & Johnson commentedThis patch (2939356-25.patch) fixes the error in the 8.6.x core.
[error] TypeError: Argument 5 passed to Drupal\locale\LocaleConfigSubscriber::processTranslatableData() must be of the type array, string given, called in /app/web/core/modules/locale/src/LocaleConfigSubscriber.php on line 152 in Drupal\locale\LocaleConfigSubscriber->processTranslatableData() (line 142 of /app/web/core/modules/locale/src/LocaleConfigSubscriber.php)
Comment #27
rzanetti CreditAttribution: rzanetti at CI&T for Johnson & Johnson commentedThis patch (2939356-25.patch) fixes the error in the 8.6.x core.