menu callbacks removed from hook_menu do not display any menus

Let's post the old backtrace.

Here is a fix for it, working on proper tests now.

Here is also a test.

Adding work-around

Are you sure that this is not set on a legacy page?

$request->attributes->has(RouteObjectInterface::ROUTE_OBJECT)

Might be better to check for '_legacy'?

Switches the check and fixes the Views test

+++ b/core/includes/menu.inc
@@ -1274,8 +1275,19 @@ function menu_tree_page_data($menu_name, $max_depth = NULL, $only_active_trail =
+  if ($request->attributes->has('_legacy')) {
+    // @todo Remove once the old router system got removed.
+    $item = menu_get_item($active_path);
+  }

Good idea!

That is ready to fly, we have test coverage and a proper fix.

Linking an similar issue: #2081963: Shortcut add/remove link on titles don't work on route only pages

+++ b/core/includes/menu.inc
@@ -1274,8 +1275,19 @@ function menu_tree_page_data($menu_name, $max_depth = NULL, $only_active_trail =
+  elseif ($route = $request->attributes->get(RouteObjectInterface::ROUTE_NAME)) {
+    $item['href'] = $request->attributes->get('_system_path');
+    // @todo What should be done on a 404/403 page?
+    $item['access'] = TRUE;
+  }

Uh, woah there now. We can't switch one critical bug for another critical security bug. We need that resolved before commit.

This isn't a security bug - it's just faking the access check and allowing the menu to render. Crell was in favor of removing this check anyhow - it's debatable as to whether it's useful.

Currently there is a regression of sorts in core that we cannot tell if the current page is a 403 or 404. That's being worked on in #2068471: Normalize Controller/View-listener behavior with a Page object so We should probably mention that issue in the todo - basically this is a fix to get things mostly right.

Improved the variable names and code comments to make it more clear what's happening.

It is a bit confusing that we don't cast just when we need it but I won't fight this.

#12: menu_tree-routing-2078583-12.patch queued for re-testing.

ok, the earlier test fail seems to have been something sporadic, since it was unrelated to the patch.

This looks fine, but could we reverse the check so the legacy is the fallback? Will make profiling more accurate the more routes are converted.

Yeah this was the route in the earlier patch as well. Additional this is easier to read.

You seem to have this already committed?

Yes I must have committed before I spotted the order. Committed/pushed the follow-up as well.