I tracked down a very hard-to-find bug in the language switcher block to an issue with how ConfigurableLanguageManager::getLanguageSwitchLinks gets its links. It iterates through the negotiation methods provided by LanguageNegotiator::getNegotiationMethods and uses the first one that supports language switch links. However, these methods are not ordered by weight - in my case, they are returned in the order language-session, language-url, even though the second one has a lower weight.

If the site is set to use language prefixes (/en/, /de/, /fr/, etc.) then this results in links like /en/node/123?language=de, which will not actually change the language as the prefix takes precedence.

Adding a weight-sort in LanguageNegotiator::getNegotiationMethods (immediately after the array_intersect) fixes this, and lets the LanguageUrl negotiator handle the switcher links, returning the correct /de/node/123 or finding language-specific aliases when available.

CommentFileSizeAuthor
#17 interdiff-16-17.txt1.29 KBKrzysztof Domański
#17 2843822-17.patch2.02 KBKrzysztof Domański
#16 2843822.patch2.04 KBborisson_
#16 interdiff-2843822.txt947 bytesborisson_
#13 2843822-13.patch1.77 KBborisson_
#12 2843822-12.patch1.77 KBborisson_
#12 interdiff-2843822.txt2.35 KBborisson_
#9 interdiff-2843822.txt2.09 KBborisson_
#9 2843822-8.patch1.74 KBborisson_
#9 2843822-9-testonly.patch1.16 KBborisson_
#3 0001-Issue-2843822-Sort-language-negotiation-methods-by-w.patch2.29 KBcburschka
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cburschka created an issue. See original summary.

cburschka’s picture

cburschka
they
CreditAttribution: cburschka at PwC's Experience Center commented
Issue summary: View changes
cburschka’s picture

cburschka
they
CreditAttribution: cburschka at PwC's Experience Center commented
Status: Active » Needs review
FileSize
0001-Issue-2843822-Sort-language-negotiation-methods-by-w.patch2.29 KB

Here is a patch and an accompanying kernel test.

(The test may need to be expanded slightly. It will definitely pass with the patch, but it might not reliably fail without the fix.)

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer commented
Status: Needs review » Needs work

The patch still applies. I ran the test without the patch in LanguageNegotiator and it was still green, so the test doesn't test what it think it's testing.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and at Calibrate commented
Status: Needs work » Needs review
FileSize
2843822-9-testonly.patch1.16 KB
2843822-8.patch1.74 KB
interdiff-2843822.txt2.09 KB

I changed the code and test to make sure that this now tests what it's supposed to.

The last submitted patch, 9: 2843822-9-testonly.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented 
+++ b/core/modules/language/tests/src/Kernel/LanguageNegotiationMethodTest.php
@@ -0,0 +1,29 @@
+class LanguageNegotiationMethodTest extends LanguageTestBase {

Could this perhaps given a more generic name such as LanguageNegotiatorTest to accommodate future tests for that class.

We could then add a @coversDefaultClass annotation too.

Other than that, this looks good to me

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and at Calibrate commented
FileSize
interdiff-2843822.txt2.35 KB
2843822-12.patch1.77 KB

Fixed those remarks.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and at Calibrate commented
FileSize
2843822-13.patch1.77 KB

Also fixed the CS error.

joachim’s picture

joachim CreditAttribution: joachim as a volunteer commented 
  1. +++ b/core/modules/language/src/LanguageNegotiator.php
+++ b/core/modules/language/src/LanguageNegotiator.php
@@ -199,6 +199,7 @@ public function getNegotiationMethods($type = NULL) {

    Should the documentation for this be changed to say that they are sorted? 

  2. +++ b/core/modules/language/src/LanguageNegotiator.php
@@ -199,6 +199,7 @@ public function getNegotiationMethods($type = NULL) {
+    uasort($definitions, '\Drupal\Component\Utility\SortArray::sortByWeightElement');

    Use

    uasort($info, [SortArray::class, 'sortByWeightElement']);

    so the class can be imported.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and at Calibrate commented
Status: Needs review » Needs work

I don't think we should update the documentation (#14.1). This would mean we'd have to either update the interface, and we're not sure that all implementations do the sort or copy the docs to this class.

The other change is valid, we should do that.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and at Calibrate commented
Status: Needs work » Needs review
FileSize
interdiff-2843822.txt947 bytes
2843822.patch2.04 KB
Krzysztof Domański’s picture

Krzysztof Domański
Location Poland
CreditAttribution: Krzysztof Domański at abventor commented
FileSize
2843822-17.patch2.02 KB
interdiff-16-17.txt1.29 KB

Small fix.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

idebr’s picture

idebr CreditAttribution: idebr at iO commented

Closed #2906210: Languages detection order are not preserved (Language doesn't switch) as a duplicate issue.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented
Status: Needs review » Closed (outdated)
Issue tags: +Bug Smash Initiative
Related issues: +#3007386: Language negotiator weights should be changeable in settings.php

I tested this on Drupal 10.0.x, demo_umami install and was not able to reproduce this error. I followed the steps given in the Issue Summary.

Therefore, closing as outdated.

If you are experiencing this problem on a supported version of Drupal reopen the issue, by setting the status to 'Active', and provide complete steps to reproduce the issue (starting from "Install Drupal core").

Adding related issue, #3007386: Language negotiator weights should be changeable in settings.php which has a similar solution.

Thanks!