Problem/Motivation

Currently the NodeListBuilder::buildRow() method tries to set the correct language for the node link. But before doing this, it checks if the language is available:

$options += ($langcode != LanguageInterface::LANGCODE_NOT_SPECIFIED && isset($languages[$langcode]) ? ['language' => $languages[$langcode]] : []);

Unfortunately $languages is never initialized in this method, so the isset($languages[$langcode]) part of the condition is always FALSE.

Proposed resolution

  • Initialize $languages variable with \Drupal::languageManager()->getLanguages()

Remaining tasks

  • Provide a patch to fix this issue

User interface changes

n/a

API changes

n/a

Data model changes

n/a

CommentFileSizeAuthor
#13 2935654-13.patch1.85 KBlongwave
#11 2935654-11.patch1.54 KBlongwave
#2 2935654-2.patch1.09 KBhctom
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hctom created an issue. See original summary.

hctom’s picture

hctom
Primary language German
CreditAttribution: hctom at undpaul commented
Status: Active » Needs review
FileSize
2935654-2.patch1.09 KB

... and here is the proposed patch to fix this issue.

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Nice, find, and the patch looks great.

I guess we now need to test that the node title link gets the language option set.

Switching to needs work for that test.

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.

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.

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.

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.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
2935654-11.patch1.54 KB

Rediscovered this following #3178534: Start running PHPStan on Drupal core (level 0)

I think we can just drop this code, as in the attached patch. EntityBase::toUrl() already sets the language of the URL:

    // Display links by default based on the current language.
    // Link relations that do not require an existing entity should not be
    // affected by this entity's language, however.
    if (!in_array($rel, ['collection', 'add-page', 'add-form'], TRUE)) {
      $options += ['language' => $this->language()];
    }

Therefore NodeListBuilder doesn't have do anything additional here, as the language option is already set correctly, and this is just dead code that can be removed.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Issue tags: +Bug Smash Initiative
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
FileSize
2935654-13.patch1.85 KB

Fix unused use statement.

joachim’s picture

joachim CreditAttribution: joachim at Factorial GmbH commented
Status: Needs review » Reviewed & tested by the community

LGTM.

hctom’s picture

hctom
Primary language German
CreditAttribution: hctom at undpaul commented

Looks good to me as well! I guess existing test coverage should be enough to ensure this change does not break anything.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed e41394e and pushed to 9.3.x. Thanks!

As there is little risk of disruption here, backported to 9.2.x

  • larowlan committed 647ad66 on 9.2.x 
    Issue #2935654 by longwave, hctom: Use of undefined $languages variable...
  • larowlan committed e41394e on 9.3.x
    Issue #2935654 by longwave, hctom: Use of undefined $languages variable...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.