Follows #3624762: Top bar entry for the main content display: bundle display or override.
That issue removes our per-view-mode "Display: …" local tasks from canonical pages, because the Display dropdown in the top bar already lists the display the page renders. The check is on the Navigation module being installed:
if (!$this->moduleHandler->moduleExists('navigation')) { return; }
But core only builds the top bar for users with the "access navigation" permission (NavigationRenderer::buildTopBar()). A user who may edit displays without that permission therefore loses the tabs and gets no dropdown in exchange: the entity's overrides become unreachable from its page.
Remove the tasks only when this user actually gets the top bar: check "access navigation" alongside the module, and add user.permissions to the cacheability that core hands the hook, so the local tasks are not cached across both kinds of user.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 3625978 after.png | 347.84 KB | csakiistvan |
| #8 | 3625978 before.png | 339.83 KB | csakiistvan |
Show commandsStart 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:
changes
Comments
Comment #3
mogtofu33 commentedComment #4
pdureau commentedbeta9 has been released. Changing the tag.
Comment #5
pdureau commentedI wanted to give a help by rebasing the MR, but I see it contains only the changes from #3624762: Top bar entry for the main content display: bundle display or override (with some tiny differences).
So, if I give a try, it would be in a distinct MR.
Seems straightforward now #3624762: Top bar entry for the main content display: bundle display or override is merged.
Maybe just adding
$cacheability->addCacheContexts(['user.permissions'])and aAccountInterface::currentUser->hasPermission('access navigation')check inDisplayBuilderEntityViewHook::menuLocalTasksAlter()would be enough.Comment #7
pdureau commentedMr ready for review. It was a bit too "simple". I will do some manual checks before sending to someone else for review.
Comment #8
csakiistvan✅ Tested and works — as a user with every permission except "access navigation", a node with overridable displays showed no top bar and no "Display: …" tabs; after applying MR !520 and rebuilding caches, the "Display: Full" and "Display: Teaser" tabs are back and open without access denied. Users without the top bar can reach the overrides again. Suggest to RTBC
Notes
user.permissionsis already a required render cache context (core.services.yml,renderer.config), so the new test still passes withoutaddCacheContexts(['user.permissions']); its docblock claims more than it covers.agents/entity-view-overrides.md(line 32) still says the Display dropdown replaces the tabs on the canonical page, without the "access navigation" condition.Comment #9
pdureau commentedThanks you, I will remove this (and update the doc), and send back to review.
Comment #10
csakiistvanJust ping me and I will review again
Comment #11
pdureau commentedI changed my mind. I finally prefer to declare explicitly in
menu_local_tasks_alterevery context the renderable depends on, without depending on a default value from the Core Renderer (whcih can change, because overridable in aservices.yml).Core does the same in
AccessResult::allowedIfHasPermission()Back to RTBC.