Configuration language being overwritten during module install

Created on 29 August 2017, over 6 years ago
Updated 21 February 2024, 10 days ago

Problem/Motivation

Steps to reproduce

  • Fresh install (in english)
  • Enable the locale and language modules.
  • Add a language (dutch in my case)
  • Make the newly added language standard language
  • Enable a module which provides default config (book module for example)

Note that this bug only occurs if the default language or the fallback language is not English.

Result

In the UI, you will see some English instead of Dutch on various pages that load configuration.

In the database:
- The base config items in the config table now have a langcode element in them, telling Drupal they are in Dutch not English.
- So when you try to load config, the base config thinks it is Dutch (even though the text is English), and the config overrides (which are still in the config database) are not being loaded to translate the English into Dutch.

Expected result

The English configuration is interpreted as English config, not Dutch, and the config translation overrides are used when viewing the site in Dutch.

Related issues

An issue which does a great job of explaining the issue, including a reference to core issue this was introduced. It also provides proposals: โœจ Configuration langcode is forced to site default language Needs work

This issue seems fairly similar to ๐Ÿ› Prevent saving config entities when configuration overrides are applied Needs work .

Also, UI text gets overridden when modules are installed. This issue is about config being mangled; the UI text problem is a separate issue: ๐Ÿ› There is no indication on configuration forms if there are overridden values Needs work and solved ๐Ÿ› Installing a module causes translations to be overwritten Fixed .

Proposed resolution

There was some debugging done; probably the cause is described in #6 / #11 / #14

There was a test-only patch in #5 but it's really for the related UI text issue, not the config text issue.

Remaining tasks

Make a patch, including a test.

User interface changes

Configuration language will not be changed to an inappropriate language.

API changes

None.

Data model changes

None.

๐Ÿ› Bug report
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Localeย  โ†’

Last updated about 6 hours ago

Created by

๐Ÿ‡ง๐Ÿ‡ชBelgium geertvd

Live updates comments and jobs are added and updated live.
  • Triaged core critical

    There is consensus among core committers that this is a critical issue. Only core committers should add this tag.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates : some issue and comment data are missing.

  • ๐Ÿ‡ซ๐Ÿ‡ทFrance andypost

    Updated summary with remaining UI related ๐Ÿ› There is no indication on configuration forms if there are overridden values Needs work

    and replaced fixed issue with ๐Ÿ› Prevent saving config entities when configuration overrides are applied Needs work

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia shalini_jha

    Hello everyone,

    I attempted to reproduce this issue on my local environment, and I successfully replicated it by following these steps:

    step 1) Freshly installed Drupal.
    step 2) Installed essential multilingual modules such as locale, content_translation, config_translation, and language.
    step 3) Added a new language, Dutch, through the admin interface (admin/config/regional/language),and added dutch as a default.
    step 4) downloaded Dutch translations from localize.drupal.org.
    step 5) Went to /admin/config/regional/translate/import and imported the translation file (.nl.po extension).
    step 6) Examined the configuration of a content type (in this case, the Article content type) and confirmed that all values were stored in Dutch (see screenshot: article content type config.png).
    step 7) Installed a module with default configurations; for example, I tried the Book module.
    Step 8)After installing the Book module, I tested the configuration of the Book content type and noticed that it forcefully overrides the configuration language to Dutch (see screenshot: book content type configuration.png).

    Upon investigation, I found that the issue is related to the locale_system_set_config_langcodes function in local.module. The function calls updateDefaultConfigLangcodes, which contains the following code:

    $default_langcode = $this->languageManager->getDefaultLanguage()->getId(); //this will provide selected default language in site.
    $langcode = $config->get('langcode'); // this will provide the configuration language like in book module have "en".
    if (empty($langcode) || $langcode == 'en') {
       $config->set('langcode', $default_langcode)->save();
    }
    

    This code forcefully sets the configuration language to the default site selected language whenever a module or theme is installed
    This behavior seems to be affecting the Book module, where the configuration language code is 'en' as actual language.

    I have added a logger in tracing the execution flow and identifying the specific language code being added to the Book module configuration. Notably, the logger revealed that the Book module's configuration is assigned the language code 'en' (see screenshot: book config selected languagecode.png).

  • Status changed to Needs review about 1 month ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia shalini_jha

    I have taken reference from the #38 , as it prevent the config overrides to default language to default config file of module. as i have tested this book module only after applying this path book module default config value is not overrides with dutch language its in English only.
    i have added a patch against 11.x .
    Please review.

  • Pipeline finished with Failed
    about 1 month ago
    #81690
  • Pipeline finished with Failed
    about 1 month ago
    #81702
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia shalini_jha

    i have added a MR with test. please review.

  • Status changed to Needs work about 1 month ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    MR appears to have a test failure.

    Added some nitpicky comments to the MR that should be addressed too while tests are updated.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 553s
    #83940
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia shalini_jha

    Addressed the mentioned comment, please review.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom joachim

    I wonder if the test be a kernel test rather than a browser test? I'm going to experiment with converting it :)

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom joachim

    With the current MR, going to admin/config/regional/language crashes with:

    > Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException: Circular reference detected for service "router.route_provider", path: "options_request_listener -> router.route_provider -> cache_tags.invalidator -> maintenance_mode_subscriber -> url_generator". in Drupal\Component\DependencyInjection\Container->get() (line 147 of /var/www/html/repos/drupal/core/lib/Drupal/Component/DependencyInjection/Container.php).

  • Pipeline finished with Failed
    19 days ago
    Total: 165s
    #93554
  • Pipeline finished with Failed
    18 days ago
    Total: 176s
    #93740
  • Pipeline finished with Failed
    18 days ago
    Total: 162s
    #93786
  • Pipeline finished with Failed
    18 days ago
    Total: 189s
    #93793
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom joachim

    I can't work out how the translated node type label is supposed to show when you go to de/admin/structure/types/manage/page -- it doesn't work manually for me at all.

    So I can't figure out how the actual test part of the test could be done using a kernel test, as I can't work out what's happening when it goes wrong. How does the translated node type label get loaded to be shown in the form?

    In the meantime, I'm streamlining the Functional test with code from the kernel test I started writing.

    I'm also removing the mentions of locale_test_translate -- that's not being enabled in this test, so it looks like frankencode comments to me :)

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom joachim

    I wonder whether the kernel tests in LocaleConfigSubscriberForeignTest are relevant here -- they seem to be testing the same sort of things.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom joachim

    Ok I am now fairly sure that test coverage for this should be added to LocaleConfigSubscriberForeignTest. But I am very confused about how to make that work:

      public function testInstallModuleWithConfiguration() {
        // Install the Language module's configuration so we can use the
        // module_installer service.
        $this->installConfig(['language']);
        $this->container->get('module_installer')->install(['locale_test_translate']);
        $this->installConfig(['locale_test_translate']);
    
    
        // Do we need to do this?
        locale_system_set_config_langcodes();
        $langcodes = array_keys(\Drupal::languageManager()->getLanguages());
        $names = Locale::config()->getComponentNames();
        Locale::config()->updateConfigTranslations($names, $langcodes);
    
        // Not this -- it fails on both 11.x and the feature branch.
        $this->assertEquals('hu', \Drupal::service('locale.config_manager')->getDefaultConfigLangcode('locale_test_translate.settings'));
    
        // ???
        $this->assertEquals('hu', $this->configFactory->getEditable('locale_test_translate.settings')->get('langcode'));
    

    My problem is that I don't understand the underlying architecture of how config is translated to understand what all the test helpers do and what the services like localeConfigManager / LocaleTranslation etc do. I suspect this is the case with most people and that the translation system has a very low bus factor!

  • Pipeline finished with Canceled
    11 days ago
    #99504
  • Pipeline finished with Failed
    11 days ago
    Total: 175s
    #99505
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom joachim

    I'm digging some more in this.

    I'm confused by the fix in the MR being in code that says this:

          // Update active configuration copies of all prior shipped configuration if
          // they are still English. It is not enough to change configuration shipped
          // with the components just installed, because installing a component such
          // as views may bring in default configuration from prior components.
    

    'prior shipped configuration' means config that was installed from a module *before* the current install operation. So I don't see how that's going to help with the bug, which is about something that goes wrong with config from a module *as it's being installed*.

    But then I'm not sure what updateDefaultConfigLangcodes() is trying to do anyway -- what does 'default' mean in this context? In getDefaultConfigLangcode(), 'default' means 'shipped with a module's code in config/install'. But here we're updating config, so it's not the shipped version is it?

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia shalini_jha

    Whenever a module with configuration settings is installed, the configuration is consistently overridden by the default language of the site if it is not set to English. The updateDefaultConfigLangcodes function is tasked with updating the language code of the configuration provided by that module. To mitigate this issue, I have implemented a check to verify whether the language code of the current configuration contains 'en' (English) and if 'en' is already included in the available language codes. This check prevents the configuration from being overridden by the default selected language.

  • Pipeline finished with Failed
    10 days ago
    Total: 160s
    #100694
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom joachim

    It would be really good if the code comment explained that a bit more, and more precisely, explained the circumstances under which the language code should be updated.

    I think this logic could be made clearer:

              $langcode = $config->get('langcode');
              $is_available_langcode = in_array($langcode, $available_langcodes);
              if ((empty($langcode) || $langcode == 'en') && !$is_available_langcode) {
    

    In the case that $langcode is empty, then it's obviously not in the array. So we only need to check if it's $available_langcodes if it's 'en', AFAICT?

    So for example, this would be clearer:

              if (empty($langcode) || ($langcode == 'en') && !$is_available_langcode) {
    

    And while I am generally REALLY in favour of splitting up complex conditionals, I'm not sure breaking out the check for $is_available_langcode is good for readability here, and it's not efficient, as it's checking even if $langcode is empty, or not 'en'.

    I'm still not sure how we test the patch using the API.

    I've installed book when 'fr' is the default language, with and without the fix, and looked at the {config} table.

    Without fix:

    	node.type.book	a:12:{s:4:"uuid";s:36:"10752ba3-9235-4540-a959-c9e9190376ae";s:8:"langcode";s:2:"fr";s:6:"status";b:1;s:12:"dependencies";a:1:{s:8:"enforced";a:1:{s:6:"module";a:1:{i:0;s:4:"book";}}}s:5:"_core";a:1:{s:19:"default_config_hash";s:43:"xvVZ9piiDxh4ziNyl-YqCT5vF8nI7xyupTdQWlN-Hxk";}s:4:"name";s:9:"Book page";s:4:"type";s:4:"book";s:11:"description";s:87:"<em>Books</em> have a built-in hierarchical navigation. Use for handbooks or tutorials.";s:4:"help";N;s:12:"new_revision";b:1;s:12:"preview_mode";i:1;s:17:"display_submitted";b:1;}
    language.en	node.type.book	a:2:{s:4:"name";s:9:"Book page";s:11:"description";s:87:"<em>Books</em> have a built-in hierarchical navigation. Use for handbooks or tutorials.";}
    

    With fix:

    	node.type.book	a:12:{s:4:"uuid";s:36:"88e05137-2a0e-4a18-840e-60769f3dccd7";s:8:"langcode";s:2:"en";s:6:"status";b:1;s:12:"dependencies";a:1:{s:8:"enforced";a:1:{s:6:"module";a:1:{i:0;s:4:"book";}}}s:5:"_core";a:1:{s:19:"default_config_hash";s:43:"xvVZ9piiDxh4ziNyl-YqCT5vF8nI7xyupTdQWlN-Hxk";}s:4:"name";s:9:"Book page";s:4:"type";s:4:"book";s:11:"description";s:87:"<em>Books</em> have a built-in hierarchical navigation. Use for handbooks or tutorials.";s:4:"help";N;s:12:"new_revision";b:1;s:12:"preview_mode";i:1;s:17:"display_submitted";b:1;}
    

    I can see there's a difference, obviously -- only one row rather than two!

    But shouldn't I be seeing translated text in there?

    I also don't understand what the API reports.

    I've got this debug code:

    $config_name = 'node.type.book';
          // enabled but not default language
          $override = $this->languageManager->getLanguageConfigOverride('en', $config_name);
          dump($override->isNew());
    
          // the default language
          $override = $this->languageManager->getLanguageConfigOverride('fr', $config_name);
          dump($override->isNew());
    

    WITH the fix, I get FALSE, TRUE

    WITHOUT the fix, I get TRUE, TRUE

    I don't understand how BOTH are reporting they're new without the fix.

Production build https://api.contrib.social 0.61.6-2-g546bc20