Problem/Motivation

Since core issue #1578832, MenuActiveTrail::getActiveLink() also loads menu links whose route is <front> when the current request is the front page.

Admin Toolbar Tools defines the "Front page" link (the Drupal logo item, admin_toolbar_tools.help) in the admin menu with route_name: <front>. As a result, on the front page this link is now considered the active link in the admin menu, and the admin menu's active trail becomes Administration › Front page.

Note: this link was labeled "Tools" before #3556081 (between 3.6.2 and 3.6.3).

This is visible with modules that build breadcrumbs from the active trail. For example, with Menu Breadcrumb configured to use the Administration menu, the front page breadcrumb for users with access to the admin menu gets extra "Administration › Front page" items, instead of no breadcrumb or just "Home".

Steps to reproduce

  1. Install Drupal 11.x (a version that includes #1578832), Admin Toolbar 3.6.x and Admin Toolbar Tools.
  2. Install Menu Breadcrumb and enable the "Administration" menu in its settings (/admin/config/user-interface/menu-breadcrumb).
  3. Make sure the main menu (or any menu with a higher priority in the Menu Breadcrumb settings) has no link to the front page.
  4. Log in as an administrator and visit the front page.
  5. The breadcrumb ends with "Administration › Front page" (or "Administration › Tools" before #3556081).

Notes:

  • The first breadcrumb ("Home") appears or not depending on the Menu Breadcrumb configuration.
  • The "Administration" breadcrumb (/admin) appears even if the user does not have access to it.

Proposed resolution

Define the "Front page" link with url: 'internal:/' instead of route_name: <front>.

  • Static menu links defined with url are stored with an empty route_name in menu_tree, so loadLinksByRoute('<front>') no longer matches them.
  • Url::fromUri('internal:/') is converted to the routed <front> URL: the link still renders as a link to the site root, with the language prefix (/en, /es) and the access check unchanged, so the toolbar looks and behaves the same.
  • The menu links documentation describes url as intended for external links, and Url::fromUri() discourages internal: when a route is available, but it works as expected. A comment in the YAML file explains the choice.
  • Backward compatibility: url menu links and the internal:/ scheme have been supported since Drupal 8. Cores without #1578832 are not affected: the link behaves as before.

Tests: AdminToolbarToolsFrontLinkTest (kernel) checks that the link is still defined in the admin menu and points to the site root, that it is no longer loaded for the <front> route, that menu.active_trail returns no active link on the front page, and that the active trail still works on an admin page. The tests fail without the change.

Remaining tasks

  • Review and merge the merge request.

User interface changes

None.

API changes

None.

Data model changes

None. Existing sites need a cache rebuild (or drush updb, which clears caches) so the menu link definition is re-saved.

AI disclosure: I've used Claude Code to help generate the boilerplate for this bug.

Issue fork admin_toolbar-3634150

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

apugacescu created an issue. See original summary.

apugacescu opened merge request !226

apugacescu’s picture

apugacescu commented
Status: Active » Needs review

Added proposed solution.

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented
Issue summary: View changes

I am cleaning up whitespace in the issue summary.

@apugacescu and I are working on the same project. I also used Claude to figure out why Tools was showing in the breadcrumb and to explore options for fixing the problem.

dydave’s picture

dydave commented

Thanks a lot everyone for your great help on this. 🙏

@everyone:

I checked the patch, it's very small and I have a question before merging this:

What are the technical implications of such a change for older versions, such as D9.5?
D10?
Is it going to work the same, as expected?

Thanks in advance!

dydave’s picture

dydave commented

@benjifisher: You're OK with the patch?

I completely trust your opinion.

I have not tested ... just reviewed and the changes looked fine to me. 👌

As soon as I can have your confirmations, I'll get this merged in ASAP.

Thanks so much!

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented

@dydave:

Thanks for the vote of confidence. I have to be careful now to get it right. ;)

First, a couple of comments about the steps to reproduce in the issue summary.

  • After #3556081: Admin Toolbar Tools: Remove dependency on Admin Toolbar (between 3.6.2 and 3.6.3) you will see "Front page" instead of "Tools" as the last breadcrumb.
  • The first breadcrumb ("Home") appears or not depending on the Menu Breadcrumb configuration.
  • The "Administration" breadcrumb (/admin) appears even if you do not have access.

Does it work? I tested with Drupal 9.5 (PHP 8.1) and 11.x (PHP 8.5) and did not see any problems. There is a lot to test with the breadcrumbs, but as far as this module is concerned, the only question is where the top-level link (admin_toolbar_tools.help) goes. With or without the change in this issue, it goes to the home page.

According to Providing module-defined menu links,

  • route_name - used for internal links. The route name that the link points to. If this is not set, the url key must be set.
  • url - used for external links. This must be a fully qualified URL. If this is not set, the route_name key must be set.

That is, you have to provide either route_name or url (not both). (According to my mathematical training, the docs allow setting both.) One is intended for internal links and one for external links, but testing shows that url: 'internal:/' works. In fact, one example on that page is url: "internal:#top".

For the internal: scheme, see the API docs for Url::fromUri(). That also discourages you from using it when there is a route available, but it still works.

Possibly the code will be updated some day to match the documentation: then "discourages" will turn into "does not allow". But I think a lot of things would break if that happened. I think the change here is OK.

AI disclosure: I used Claude when looking at this issue during my day job, but I wrote this comment the old-fashioned way, including the testing and RTFM that backs it up.

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented

I also tested with the Umami demo profile, a multilingual site (Drupal 11.x). On an English page, the link goes to /en, and on a Spanish page it goes to /es. That is true for both the current 3.x branch of this module or the feature branch for this issue.

dydave’s picture

dydave commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Thanks a lot @apugacescu for the report and the merge request, and @benjifisher for the detailed review and testing! 🙏

I confirmed the cause: since #1578832, MenuActiveTrail::getActiveLink() also loads the links with the <front> route on the front page, and admin_toolbar_tools.help was defined with route_name: <front>.

Regarding compatibility (D9.5 / D10): menu links with url and the internal:/ scheme have been supported since D8, and Url::fromUri('internal:/') is converted to the routed <front> URL, so the link rendering, the language prefix and the access check are the same. Cores without #1578832 are not affected.

I added two things to the merge request:

  • A comment in the YAML file explaining why url is used instead of route_name.
  • A kernel test, AdminToolbarToolsFrontLinkTest, with its CI configuration. It fails without the change (the link is loaded for the <front> route and is the active link on the front page) and passes with it.

Existing sites need a cache rebuild (or drush updb) so the menu link definition is re-saved.

I also updated the issue summary with the corrections from comment #7 (link label, breadcrumb steps).

Setting to RTBC once the pipeline is green. 👍

  • dydave committed 63f337df on 3.x authored by apugacescu 
    Issue #3634150 by apugacescu, benjifisher, dydave: Fixed the 'Front page...
dydave’s picture

dydave commented
Status: Reviewed & tested by the community » Fixed

Merged, thanks a lot @apugacescu for the report and the fix, and @benjifisher for the thorough review and testing! 🙏

The 'Front page' link is now defined with a URL, so it is no longer the active link of the admin menu on the front page. A kernel test covers it. Existing sites need a cache rebuild (or drush updb) so the menu link is re-saved.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.