Problem/Motivation
Found this while working on #2212069: Non-English Drupal sites get default configuration in English, edited in English, originals not actually used if translated. When config entities get imported from shipped defaults, they are resaved after instantiated in the import and therefore the undefined language default in ConfigEntityBase gets applied. So if the original shipped file did not include a language code, the entity will now (incorrectly) be undefined language. While configuration otherwise assumes English defaults if the langcode was not specified (see LocaleConfigManager::get(), ConfigNamesMapper::getLangcode(), etc.), this makes the configuration now *know* undefined instead of keeping to assume English.
We also cannot assume lack of language on ANY config entity because even RDF mappings, and view/form displays which do not have any human language text on them may take third party settings, which are impossible to predict if they would contain human language content or not. So unless we want to require people to set some concrete langcode on config entities when setting a third party setting that includes human language content, we need to keep assuming that all config entities are in a human language (and not 'und' or NULL).
Proposed resolution
Option 1. Fix the concrete files in core and document that config entities will not have the same missing langcode fallback at least while being imported.
Option 2. Fix the import code to add merge langcode: en to loaded default configuration before instantiating entities.
Option 3. Change the default in ConfigEntityBase to English and document it.
Patch implements Option 3 for now.
Remaining tasks
Review.
User interface changes
None.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
#17 | 2451885-17.patch | 83.07 KB | Gábor Hojtsy |
#17 | interdiff.txt | 3.15 KB | Gábor Hojtsy |
#14 | interdiff.txt | 1.66 KB | Gábor Hojtsy |
#14 | 2451885-14.patch | 81.75 KB | Gábor Hojtsy |
#11 | 2451885-11.patch | 80.09 KB | Gábor Hojtsy |
Comments
Comment #1
Gábor HojtsyComment #2
Gábor HojtsyComment #3
effulgentsia CreditAttribution: effulgentsia at Acquia commentedIf ConfigNamesMapper::getLangcode() defaults to English, then seems to me that ConfigEntityBase::$langcode should as well. Which is perhaps an option 3 (or 2b)? Otherwise, if we want undefined to be the default, in order to not bias English, then I think we should do so everywhere (e.g., change ConfigNamesMapper accordingly), and then do option 1.
Comment #4
Gábor HojtsyThe English default is supposed to be a DX helper so people don't need to include a langcode in their file. We can default to 'en' in ConfigEntityBase too and let overriding config entities to set different defaults. For example RDF mappings cannot contain a translatable (AKA human readable) string, so they make sense to default all the time to 'und'.
Comment #5
Gábor HojtsyDropped option 1 and implemented Option 3 then. No interdiff, because no code is left from the prior patch.
Comment #6
BerdirDon't we actually set the langcode to the site default now? shouldn't the default value be NULL then?
view/form displays also don't have a label, so they should be set to UND (and forced to that) as well. How do we define that? The problem is that the schema now forces that they have a langcode, even if it is not relevant for them.
Comment #8
Gábor Hojtsy@Berdir: the 'en' default is supposed to be a DX feature, so careless developers don't need to specify 'en'. We do set the langcode to the site default **when you create a new entity** (in ConfigEntityStorage::doCreate()), but the problem is with config import. When we import a config entity that lacks a langcode, the ConfigEntityBase::$langcode applies, which is 'und' now.
As a matter of fact, I realized that we cannot assume lack of language on ANY config entity because even RDF mappings, and view/form displays may take third party settings, which are impossible to predict if they would contain human language content or not. So unless we want to require people to set some concrete langcode on config entities when setting a third party setting that includes human language content, we need to keep assuming that all config entities are in a human language (and not 'und' or NULL).
Comment #9
Gábor HojtsyPatch with more than 0 bytes :D
Comment #11
Gábor HojtsyFollowing that thinking, replacing all explicit langcode: und with langcode:en then. Almost all are in test views.
Comment #12
Gábor HojtsyThis is now the only known bug with #2212069: Non-English Drupal sites get default configuration in English, edited in English, originals not actually used if translated.
Comment #13
vijaycs85documentation++
Do we want to change this to 'en'. Can't we remove the langcode itself? setting it to default en wouldn't cause any problem when installing in other language?
Comment #14
Gábor HojtsyAdding tests too checking implicit and explicit langcode in config entity imports.
@vijaycs85: so long as the default configuration (may) contain English strings, it should be marked English, not a problem with foreign language installs at all. See more at #2212069: Non-English Drupal sites get default configuration in English, edited in English, originals not actually used if translated.
Comment #15
vijaycs85Test looks good and ready to go.
Comment #17
Gábor HojtsyLet's add the new test config to a new module so it does not affect existing tests, ha.
Comment #18
effulgentsia CreditAttribution: effulgentsia at Acquia commentedSeems sensible. Back to RTBC.
Comment #19
effulgentsia CreditAttribution: effulgentsia at Acquia commentedLeaving this issue at RTBC, because I think the code changes in the patch make sense.
But, regarding this doc:
See #2453551-4: TranslationLanguageRenderer tries to add langcode field to the view for entity types that have no langcode.
If based on further discussion we need to change the doc, we can do so in a follow-up.
Comment #20
Gábor Hojtsy@effulgentsia: Config entities at least if they extend from ConfigEntityBase all have a langcode. Unlike content fields themselves, which do have their own langcode (as related to the discussion in #2453551: TranslationLanguageRenderer tries to add langcode field to the view for entity types that have no langcode), third party settings have no way to specify their langcode, they just inherit the langcode provided on the config file level.
Comment #21
alexpottThis issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed eaffcfc and pushed to 8.0.x. Thanks!
Comment #22
Gábor HojtsyThanks, yay!