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.
I figured out a bug in the theme_links() function for menu links. I found the bug http://drupal.org/node/173459 and solved this bug the same way.
Repro:
1. Add a menu link with path <front>
to your primary links
2. Add some more menu link, but this is not required - this is only to verify the bug
3. Click on the <front>
link to get it active
4. See source what is marked active and you will see the link is marked 'active', but the <li>
*not* (BUG).
This makes styling of the active LI element absolutely impossible. Please get this trivial patch into for 6.2... it bugs me any my theme users.
Comment | File | Size | Author |
---|---|---|---|
#2 | theme_link_broken_li_active-HEAD.patch | 716 bytes | hass |
theme_link_broken_li_active.patch | 714 bytes | hass | |
Comments
Comment #1
Damien Tournoud CreditAttribution: Damien Tournoud commentedThat bug was fixed by #173459: If <front> is added to menu, the link is never marked 'active' in DRUPAL-5, but was never ported to DRUPAL-6. Let's do not make that mistake again: bugs have to be fixed in HEAD, than back-ported.
Could you re-roll the same patch for HEAD?
Comment #2
hass CreditAttribution: hass commentedIt haven't been documented, but the bug in #173459 is fixed in D6. This bug is new and related to LI only not the link.
See "common.inc":
Attached patch is rolled for D7, above for D6.
Comment #3
Damien Tournoud CreditAttribution: Damien Tournoud commentedHum, ok. Thanks for the clarification.
The above patchs only do implement the solution of #173459 in
theme_links()
, so marking this as ready to go in. Is there other parts of the core where this could be needed?Comment #4
hass CreditAttribution: hass commentedI cannot say for sure if there is more broken... i only know #173459 is fixed and struggled on trying to style an LI missing the 'active' class. THX for review.
Comment #5
Gábor Hojtsydamz: http://drupal.org/node/173459 you linked to (which adds an extra check into l()) is also fixed in Drupal 6, l() also includes that code there.
Committed to 6.x. Still needs to go to 7.x.
Comment #6
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #7
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.