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.
While working on #2224861: Cache SystemMenuBlock and BookNavigationBlock per active trail (currently cached per URL, this breaks on very long URLs), pwolanin, dawehner and I noticed that all of the menu_set_active_trail()
/menu_get_active_trail()
code was dead. I don't know the menu system well at all, nor its evolution in D8, but AFAICT it's because all of the active trail handling has been moved into MenuTree
and classes implementing BreadCrumbBuilderBaseInterface
(which use metadata in the routing system). Therefore, nothing uses the old active trail system anymore.
Comment | File | Size | Author |
---|---|---|---|
#1 | dead_menu_system_code-2237963-1.patch | 20.69 KB | Wim Leers |
Comments
Comment #1
Wim LeersI was able to run all of the Menu tests and PHPUnit tests, and everything was still green :)
Only deletions! :D
Comment #2
pwolanin CreditAttribution: pwolanin commentedIs it all really dead? Maybe the tests should be using the menu links code instead to set the active trail or preferred link?
Comment #3
Wim LeersAs far as I could tell: yes.
\Drupal\menu_link\Tests\MenuTreeTest
contains unit test coverage for active trail handling.It's possible I missed something though. I tried to decipher what's going on, but it's extremely hard to understand what's going on if you were never involved with the menu system at all — IMO.
Comment #4
dawehner00c06fcfedb19f1e1f86016480b29c06330972c4 removed the last real usage of that function as it was used here for the breadcrumb which is now just path based or whatever you want.
Comment #5
webchickYay, less code!
However, I don't see a change record about menu_set_active_trail and friends, and those are used extensively in contrib:
https://drupal.org/list-changes/published?keywords_description=menu_set_...
Also don't think this is blocking anything particularly, so moving down to normal.
Comment #6
pwolanin CreditAttribution: pwolanin commentedDraft change notice: https://drupal.org/node/2240003
Comment #7
pwolanin CreditAttribution: pwolanin commentedchange record
Comment #10
dawehner.