Follow-up to #2855786: Menu items that link to an empty administration page are not hidden

Problem/Motivation

For users with limited permissions (i.e. content editors), enabling the "access administration pages" permission makes a lot of menu items appear that they don't have access to, which show a "You do not have any administrative items." message.

Having all these links that go nowhere is not a very good UX:

A screenshot of System page without any available menu items

Steps to reproduce

  1. Cleanly install Drupal using the standard profile
  2. Log in as user 1
  3. Visit /admin/people/create
  4. Create a new user and grant them the Content editor role
  5. Open a private browser window
  6. Log in as the newly created editor
  7. Once authenticated, change the menu to its vertical orientation mode (either by clicking the toggle button on the right side of the toolbar or by shrinking your window width until the menu hits its responsive page break)
  8. Expand the Configuration menu item
  9. Observe that the System menu item is visible (it should not be) and click it
  10. You will be directed to an empty system admin menu block page with the text You do not have any administrative items.

Proposed resolution

Don't show menu items that go to empty administration pages by checking if children are accessible in addition to checking the "access administration pages" permission.

Remaining tasks

Patch
Review
Commit

User interface changes

Menu items to empty administration pages will be hidden.

API changes

N/A

Data model changes

N/A

Issue fork drupal-2856666

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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

stefan.r created an issue. See original summary.

stefan.r’s picture

Status: Needs review » Active

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Lendude’s picture

Status: Active » Closed (cannot reproduce)
Issue tags: +Bug Smash Initiative

Checked this with an 'editor' user in a clean Umami install. I only see links that I have access to, so this seems to have been fixed since this was opened.

If you feel this is still an issue, please provide some steps to reproduce this on a current version of Drupal. Thanks!

gabesullice’s picture

Issue summary: View changes
Status: Closed (cannot reproduce) » Active
FileSize
36.18 KB

I was able to reproduce this on the tip of the 9.5.x branch. I updated the issue description with steps to reproduce this bug from a clean installation.

gabesullice’s picture

gabesullice’s picture

Title: Check if children are accessible in access check for second level menu items in Management > Configuration » Hide administration menu links that do not have child links

I spent some time trying to get this working by adding an additional access check to all of the routes which use the SystemController::systemAdminMenuBlockPage controller, but I don't think that's going to be a fruitful approach. It's difficult to check access to menu links from that controller and after digging in, it felt a little hacky.

Instead, I think it might be easier and generally useful to add a new flag to menu items to indicate whether the item should or shouldn't be shown when it doesn't have any visible children. Tentatively, I'm naming this flag transitive with a default value of FALSE to maintain BC. The concept is that menu links like People and Development could be marked "transitive" since their only purpose is to transit users from a grandparent page to a child page. Thus, if they have no visible child links, they can be safely elided.

I'm trying to do that in MenuLinkTree, but I'm running into a hiccup because, at some point in the process of building a menu link tree, a particular item may not have any visible children even though one will be added later. Thus, my current code is removing items that it shouldn't because it doesn't "know" about the child that doesn't exist yet. I think I just have to spend more time with Xdebug to figure out where these missing child links are added.

A drawback of the menu link tree based approach is that the system menu block pages would still be accessible, even without any links, if a user visited the right URL directly. That feels like a separate issue that could be handled in a follow-up though. All we'd need to do would be to throw a 403 or 404 HTTP exception in the controller instead of printing out the You do not have any administrative items message.

gabesullice’s picture

Title: Hide administration menu links that do not have child links » Hide administration menu links that do not have visible child links

gabesullice’s picture

Component: system.module » menu system
Status: Active » Needs review
gabesullice’s picture

Status: Needs review » Needs work

Needs work for an update hook to add the transitive column to existing sites.

gabesullice’s picture

Status: Needs work » Needs review

Okay, I added code to update the database schema on existing sites. I was expecting to write an update hook to do this, but it seems like the default menu tree storage service handles its own schema changes, so I made my changes in there instead. This probably needs special attention.

Needs review for tests.

gabesullice’s picture

Status: Needs review » Needs work

Needs work to update the toolbar tests, which makes sense.

gabesullice’s picture

I'm trying to do that in MenuLinkTree, but I'm running into a hiccup because, at some point in the process of building a menu link tree, a particular item may not have any visible children even though one will be added later. Thus, my current code is removing items that it shouldn't because it doesn't "know" about the child that doesn't exist yet. I think I just have to spend more time with Xdebug to figure out where these missing child links are added.

To follow up on this: the issue was in the toolbar module. It does a quirky thing where it only loads the top level of the admin menu, then it loads the subtree of each of those top level items separately, rather than loading the whole tree at once. This explains why I wasn't seeing any children when I was step-debugging the first load of the admin menu (it was only loading a shallow depth).

Needs work to update the toolbar tests, which makes sense.

To clarify: it makes sense because I'm fiddling with the menu and the toolbar tests probably make some assumptions that don't hold any longer.

lauriii’s picture

Version: 9.5.x-dev » 11.x-dev
Issue tags: +Needs reroll

omkar.podey made their first commit to this issue’s fork.

omkar.podey’s picture

Assigned: Unassigned » omkar.podey
tim.plunkett’s picture

Issue tags: +Admin UX
tedbow’s picture

@gabesullice et al, great work on this but...
I have talked with @tim.plunkett and @lauriii and I (we?) think we should close this issue in favor of #296693-257: Restrict access to empty top level administration pages from 2008 which has different approach to solve the same general UX problem. See my linked command but basically I think route access a better way to solve this then menu items.

tim.plunkett’s picture

Assigned: omkar.podey » Unassigned
Status: Needs work » Closed (duplicate)

+1