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.

CommentFileSizeAuthor
#1 dead_menu_system_code-2237963-1.patch20.69 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers’s picture

Assigned: Wim Leers » Unassigned
FileSize
20.69 KB

I was able to run all of the Menu tests and PHPUnit tests, and everything was still green :)

Only deletions! :D

pwolanin’s picture

Is it all really dead? Maybe the tests should be using the menu links code instead to set the active trail or preferred link?

Wim Leers’s picture

As 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.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

00c06fcfedb19f1e1f86016480b29c06330972c4 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.

webchick’s picture

Priority: Major » Normal
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

Yay, 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.

pwolanin’s picture

Issue tags: -Needs change record

Draft change notice: https://drupal.org/node/2240003

pwolanin’s picture

Status: Needs work » Reviewed & tested by the community

change record

  • Commit f9f3e7f on 8.x by webchick:
    Issue #2237963 by Wim Leers: Remove dead menu system code:...

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1: dead_menu_system_code-2237963-1.patch, failed testing.

dawehner’s picture

Status: Needs work » Fixed

.

Status: Fixed » Closed (fixed)

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