#Triaged core critical

There is consensus among core committers that this is a critical issue. Only core committers should add this tag.
โšก๏ธ Live updates comments, jobs, and issues, tagged with #Triaged core critical will update issues and activities on this page.

Issues

The last 100 updated issues.

Activities

The last 7 days of comments and CI jobs.

  • ๐Ÿ‡ฌ๐Ÿ‡ง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.

  • ๐Ÿ‡ฎ๐Ÿ‡ณ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.

  • ๐Ÿ‡ฌ๐Ÿ‡ง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?

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