In setRequest() we have:

public function setRequest(Request $request) {
  // Pre-calculate and store values based on the request that may be used
  // repeatedly in generate().
  $this->active = array(
    'route_name' => $request->attributes->get(RouteObjectInterface::ROUTE_NAME),
    'language' => $this->languageManager->getLanguage(Language::TYPE_URL)->id,
    'parameters' => (array) $request->attributes->get('_raw_variables') + (array) $request->query->all(),
  );
}

This later comparison to the parameters don't work since $request->attributes->get('_raw_variables') is actually a ParamterBag object, not an array, and when it's not NULL we get an extra nested array from the cast.

CommentFileSizeAuthor
#5 test-and-fix-2084125-5.patch4.86 KBpwolanin
#5 2084125-3-5.increment.txt1.25 KBpwolanin
#3 test-only-2084125-3-FAIL.patch3.83 KBpwolanin
#3 test-and-fix-2084125-3-PASS.patch4.89 KBpwolanin
#2 2084125-1.patch1.06 KBpwolanin

Comments

pwolanin’s picture

pwolanin commented
Status: Active » Needs review
Issue tags: +Needs tests, +WSCCI

This simple fix should do it, but we should make sure there's a test in core that covers this properly.

pwolanin’s picture

pwolanin commented
StatusFileSize
new 2084125-1.patch1.06 KB
pwolanin’s picture

pwolanin commented
Title: Bug in active detection in LinkGenerator - need to extra array from _raw_variables ParameterBag » Bug in active detection in LinkGenerator - need to extract array from _raw_variables ParameterBag
StatusFileSize
new test-and-fix-2084125-3-PASS.patch4.89 KB
new test-only-2084125-3-FAIL.patch3.83 KB

Here's a change to the unit test to demonstrate the failure, and a combined patch.

dawehner’s picture

dawehner
Primary language German
commented

I remember when we wrote that, the issue about removing active was really active ... luckily we are still in php world.

+++ b/core/tests/Drupal/Tests/Core/Utility/LinkGeneratorTest.php
@@ -367,6 +373,8 @@ public function testGenerateActive() {
     $parameters = $request->query->all();

@@ -392,6 +400,22 @@ public function testGenerateActive() {
+    $parameters = $request->query->all();

These two variables are not used at all, so let's remove it.

pwolanin’s picture

pwolanin commented
StatusFileSize
new 2084125-3-5.increment.txt1.25 KB
new test-and-fix-2084125-5.patch4.86 KB

ok, sure.

pwolanin’s picture

pwolanin commented
Issue tags: -Needs tests

has tests now

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Thank you

dawehner’s picture

dawehner
Primary language German
commented

Note: we should try to split up all the different cases in the active test method into multiple tests. This potentially is easier to read when we choose descriptive function names, but this is out of scope of this issue.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

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