Generated with the help of an LLM, reviewed and tested by a human.

Problem/Motivation

Since 11.2.0, on sites using the Toolbar module, the contextual links "Edit" toggle is shown in the toolbar on every page for users with the access contextual links permission, including pages that have no contextual links at all, such as /admin/config. Clicking it on those pages does nothing except toggle its pressed state. Before 11.2.0, the toggle was hidden when a page had no contextual links.

Reproduced on a clean 11.x-dev install (commit 18d8ceb4) and on 11.3.19. The file is identical in 11.2.0 through 11.3.19, 11.5.0-beta1 and 11.x. The contrib Toolbar project (2.x) contains the same code.

Steps to reproduce

  1. Install 11.x with the Standard profile.
  2. Uninstall the Navigation module and install the Toolbar module (Standard installs Navigation on 11.x).
  3. Install Olivero and set it as the default theme, so front-end pages have blocks with contextual links.
  4. As the administrator, visit /user/1. The page has contextual links and the "Edit" toggle is shown, as expected.
  5. Visit /admin/config (or /admin/people, /admin/structure, /admin/content). The page has no contextual links, but the "Edit" toggle is still shown. See the attached screenshot.
  6. In the browser console on that page, Drupal.contextual.instances.count returns undefined and Drupal.contextual.instances.length returns 0.

Expected: the toggle has the hidden class on pages without contextual links.

Actual: the toggle is shown on every page.

Proposed resolution

  • Read Drupal.contextual.instances.length instead of .count.
  • In updateVisibility(), compute this.isVisible = this.contextualCount > 0 and apply it with this.$el[0].classList.toggle('hidden', !this.isVisible).
  • Have render() call this.updateVisibility() instead of toggling hidden itself, so the first render is correct when contextual links already exist.

User interface changes

The "Edit" toolbar toggle is hidden again on pages without contextual links.

CommentFileSizeAuthor
#3 3632476-contextual-edit-toggle-11.3.x-2.patch2.67 KBrbrandon
#2 core-contextual-toolbar-edit-toggle-visibility.patch2.08 KBrbrandon
image-1792683499620.png24.52 KBrbrandon

Issue fork drupal-3632476

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

rbrandon created an issue. See original summary.

rbrandon’s picture

rbrandon commented
Issue summary: View changes
Issue tags: -JavaScript +JavaScript
StatusFileSize
new core-contextual-toolbar-edit-toggle-visibility.patch2.08 KB

We are using the following patch for 11.3.19, I will open an MR off 11.x with similar code and test coverage.

rbrandon’s picture

rbrandon commented
Issue tags: -JavaScript +JavaScript
StatusFileSize
new 3632476-contextual-edit-toggle-11.3.x-2.patch2.67 KB

Updated 11.3.x patch to fix toggling bug hidden by old failure. Fixes manageTabbing() so it only limits tabbing while the toggle is visible.

rbrandon opened merge request !17537

rbrandon’s picture

rbrandon commented
Issue summary: View changes

MR is ready for review with test coverage.

rbrandon’s picture

rbrandon commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented

Could the summary be cleaned up to lose some of the AI verboseness please.

rbrandon’s picture

rbrandon commented
Issue summary: View changes

Sure, I did edit a lot and only left what was relevant. I will add some of the info here and refine it further.

Here are some notes on the origin of this bug and why the fix is constructed the way it is.

This is a regression from #3203920, which replaced the Backbone StateModel and VisualView with Drupal.contextual.ContextualToolbarModelView (core/modules/contextual/js/toolbar/contextualToolbarModelView.js in 11.2.x and 11.3.x, moved unchanged to core/modules/toolbar/js/views/ContextualToolbarModelView.js by #3618746). The port has three bugs that combine:

  1. Drupal.contextual.instances is a Proxy over an array with no get trap, but the class reads Drupal.contextual.instances.count in four places, which is always undefined. The contextualCount setter only calls updateVisibility() when the value changes, so it is never called.
  2. updateVisibility() calls this.get('contextualCount'), a Backbone Model method that does not exist on the new class. Bug 1 hides this; fixing bug 1 alone makes it throw TypeError: this.get is not a function.
  3. render() calls this.$el[0].classList.toggle('hidden', this.isVisible), but the Backbone VisualView used toggleClass('hidden', !this.model.get('isVisible')). Because isVisible stays false, every render removes the hidden class that ContextualHooks::toolbar() renders the tab with.

In addition, updateVisibility() only sets isVisible and nothing re-renders afterwards (the Backbone view re-rendered on change), so the toggle would not update when contextual links are added after the toolbar initializes.

Fixing any of these on its own makes things worse: fixing 1 alone throws in 2, and fixing 1 and 2 together hides the toggle on exactly the pages that have contextual links.

#3611072 reports the opposite symptom on 10.4+ (the toggle is hidden on cached page loads). It concludes that 11.x is unaffected because the new constructor counts existing links, but that count is the always-undefined instances.count; 11.x only appears fixed because the toggle is never hidden. Comment #18 on #3535051 also mentions bugs 1 and 2 as out of scope there.

rbrandon’s picture

rbrandon commented
Issue summary: View changes