Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
#1979468: ".active" from linkGenerator(), l() and theme_links() forces an upper limit of per-page caching for all content containing links optimized the setting of the .active class on links. Unfortunately, we missed one spot: menu_navigation_links()'s logic.
To make matters worse, the current logic is inherently wrong: on all "navigation links"-style menus (which includes the "primary" menu and the "secondary" menu, which contain the "Home" link and the "Log in/log out/register" links, respectively, out of the box) will get the .active class set precisely when they're not active!
Encountered while working on #1805054: Cache localized, access filtered, URL resolved, and rendered menu trees.
Proposed resolution
Delete the obsolete code; #1979468: ".active" from linkGenerator(), l() and theme_links() forces an upper limit of per-page caching for all content containing links will handle it for us automatically.
Remaining tasks
None.
User interface changes
None.
API changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | 2271025-1.patch | 887 bytes | Wim Leers |
Comments
Comment #1
Wim LeersAlso, local tasks have their own independent "active" handling, see
LocalTaskInterface::(set|get)Active(), so it probably should already have been deleted as part of #2004334: Separate Tabs (MENU_LOCAL_TASK) from hook_menu().The breakage itself was introduced in #2177031: Remove menu_get_item() and every use of it. .
Now we can finally lay this small chunk of code to rest :)
Comment #2
dawehnerYeah, let's remove that. This is confusing as hell if you try to figure out where the active class it set.
Comment #3
webchickGood catch. Committed and pushed to 8.x. Thanks!
Comment #4
Wim Leers