Contrib.social

Javascript error when Toolbar is vertical and admin menu has unrouted links

Open on Drupal.org →
Created on 15 May 2020, over 4 years ago
Updated 9 July 2024, 7 months ago

Problem/Motivation

If you add a menu link to the Administration menu, and that menu link has an unrouted URI, when the toolbar is oriented vertically, there is a Javascript error which prevents the buttons for child menus from appearing.

Steps to reproduce:

  1. Add menu link to administration menu at /admin/structure/menu/manage/admin/add
  2. Set menu link title to anything.
  3. Set link to any URL that is not routed in Drupal: (use https://www.google.com, for example)
  4. Set parent link to --Administration
  5. Save
  6. If toolbar is horizontal, click the arrow to change it to vertical
  7. Observe that buttons for child menus do not appear.
  8. Observe that error is logged in browser console.
Uncaught Error: Syntax error, unrecognized expression: #toolbar-link-https://www-google-com
    at Function.se.error (jquery.min.js?v=3.4.1:2)
    at se.tokenize (jquery.min.js?v=3.4.1:2)
    at se.select (jquery.min.js?v=3.4.1:2)
    at Function.se [as find] (jquery.min.js?v=3.4.1:2)
    at k.fn.init.find (jquery.min.js?v=3.4.1:2)
    at MenuVisualView.js?v=8.8.5:19
    at Array.forEach (<anonymous>)
    at n.render (MenuVisualView.js?v=8.8.5:18)
    at _ (backbone.js:371)
    at m (backbone.js:356)

Proposed resolution

Issue appears to be in \Drupal\toolbar\Controller\ToolbarController::preRenderGetRenderedSubtrees(), where in this line:

$id = str_replace(['.', '<', '>'], ['-', '', ''], $url->isRouted() ? $url->getRouteName() : $url->getUri());

$url->getUri() contain unsafe characters for DOM IDs.

In toolbar_menu_navigation_links(), unrouted URIs are hashed like so:

$id = substr(Crypt::hashBase64($url->getUri()), 0, 16);

It seems to make sense to use that in the Controller pre-render method as well.

$id = str_replace(['.', '<', '>'], ['-', '', ''], $url->isRouted() ? $url->getRouteName() : substr(Crypt::hashBase64($url->getUri()), 0, 16));

Remaining tasks

Needs reviews and tests

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

(Major and critical issues should have a snippet that can be pulled into the release notes when a release is created that includes the fix)

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Toolbar  →

Last updated 14 days ago

  • Maintained by
  • 🇫🇷France @nod_
Created by

godotislate

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇺🇸United States smustgrave

    Tried replicating following the steps in the IS on 10.2 but wasn't able to trigger the error.
    Just to confirm
    Created a link in the Admin menu
    Title is My test
    Link is https://www.google.com/
    Toolbar is vertical
    Click save
    Go horizontal no issue

    Create another link
    Toolbar is horizontal
    Click save
    Go vertical no issue.

  • Comment over 1 year ago →
    🇮🇳India omkar.podey

    What I think is that the steps are incomplete. As @smustgrave i got a similar result initially with the structure being

    But after i changed the structure in /admin/structure/menu/manage/admin for my test menu link(random link title) to be under Administration manually.

    It did break the child links in the vertical toolbar.

    I think this was the intended bug to be solved, right?

  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇮🇳India omkar.podey
  • Comment over 1 year ago →
    🇮🇳India omkar.podey
  • @godotislate opened merge request.
  • Comment over 1 year ago →
    godotislate

    @omkar.podey: Yes, that's the issue. Thank you for the correction in the steps. It's been a couple years, and I'm off that project, but we saw the issue surface when we put a link to our pattern library in the admin menu for convenience, and QA caught that the menu broke in the vertical configuration.

    Created an MR against 11.x-dev after tweaking the original patch to address feedback from #12 🐛 Javascript error when Toolbar is vertical and admin menu has unrouted links Needs work . Not sure how to write a better test. The Javascript for the toolbar is more complicated than I can write a test for. It doesn't look like the original test captures the issue correctly anymore either. I've left the PR at draft to make the fix available against 11.x-dev for anyone who needs it. I'm willing to revisit the tests if anyone can provide some guidance on how to write them.

  • Comment 7 months ago →
    🇺🇸United States esod

    Reroll for Drupal 10.2.7. I don't have time to look at the test right now.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024