Active menu links with identical paths get a duplicate "active" class for every active menu link

Heh, that is hilarious :)

Added a fix that checks if the class has been added before. I wonder though why each link gets processed multiple times.

Markup before:

Markup after:

We need some tests for this, also wondering if there would be a better way to manage those classes than a string, might make this easier?

Tests, and a better fix, that fixes the performance problem that this uncovered, and which the patch in #2 didn't address. No interdiff, because different patch.

Updated IS, to clarify the problem, and the solution in this patch.

Oh, and if that patch is confusing: $type_match was never used in HEAD; hence I only remove it without removing its usages: there were no usages :). I suspect I actually originally planned to do something similar to #4, but then forgot, and nobody noticed and it got committed anyway.

Reuploading the #4 patch, because testbot failed to detect it…

#4's test was fine, but its implementation changes were utterly wrong. This should be much better.

I uploaded them in the wrong order, but the test result speaks for itself. Back to NR.

The updated patch works as advertised, the issue summary update correctly reflects the content of the patch and the feedback of Berdir in #3 has been addressed. Setting this to RTBC.

Markup before:

Markup after:

+++ b/core/modules/system/src/Controller/SystemController.php
@@ -400,9 +397,11 @@ public static function setLinkActiveClass(array $element, array $context) {
+      $is_active = !in_array('active', explode(' ', $node->getAttribute('class')));

This line reads really weird because if the class has the active class $is_active equals FALSE. I think we should change the variable name to $add_active. Also I think we should store the result of $node->getAttribute('class') in a variable since we have to do this again later.

Thanks for the feedback, alexpott. I have updated the patch accordingly.

I think that addresses @alexpott's feedback correctly, agreed that $add_active is a better name.

+1

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed a5bbb06 and pushed to 8.0.x. Thanks!