Problem/Motivation

If you test a page in the admin ui with ARC Toolkit it finds duplicate IDs for the top level menu items Live (for the workspaces module) and Help . Each has the ID navigation-link-. That is not in line with WCAG2.2 SC4.1.2. The problem only applies to the default Live workspace. If you switch to another the problem does not apply and there are unique IDs.

*Was not sure against which component i should file this issue. but i went with the navigation module for now. please reassign if there is a more suitable component.

Steps to reproduce

  • Do a fresh site install.
  • Leave caching and aggregation enabled like normal
  • Enable the Navigation and the Workspaces UI modules
  • Go to for example admin/modules
  • Check id attribute on the the li elements wrapping the top level menu items Live (the default workspace) and Help

Proposed resolution

Use unique IDs for the top level menu items Live and Help

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#10 issue-3570505-10-empty-plugin-ids-exposed.png103.58 KBkentr
#7 Administration___Drush_Site-Install_and_navigation-menu_html_twig_—_drupal.png358.79 KBmherchel

Issue fork drupal-3570505

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

rkoller created an issue. See original summary.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

Had Zoom with @mgifford, @rkoller, and @kat-shaw.

I'm having difficulty reproducing this. The |clean_unique_id is adding a number at the end of the ID, so they're different.

mgifford’s picture

mgifford
he/him
Primary language English
commented

In discussion with @mherchel @rkoller & @kat-shaw we decided that this was a barrier if we can replicate it. We were having trouble tracking down the source of the problem.

This would fall under
https://www.w3.org/WAI/WCAG21/Understanding/name-role-value.html

rkoller’s picture

rkoller
he/him
Primary language German
Location Nürnberg, Germany
commented

I was able to reproduce with the main-dev branch. did a composer create project. one detail to note. on the very first page load when there is no caching yet checking the list element wrapping the menu item for the workspaces module the ID is correctly navigation-link---5 on any subsequent request the ID becomes navigation-link-. If you do a drush cron the first page load right after the ID becomes navigation-link---5 again for the list element wrapping the menu item for the workspaces module for one page request. then it is back to navigation-link-.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

This looks like a bug in |clean_unique_id.

That being said, we can work around it with random()

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

I am able to reproduce this exactly like @rkoller in #4.

I wasn't able to reproduce prior because I generally have all caching disabled.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
StatusFileSize
new Administration___Drush_Site-Install_and_navigation-menu_html_twig_—_drupal.png358.79 KB

Pushed a commit that uses Twig's random() function instead of the |clean_unique_id filter.

mherchel opened merge request !14732

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Title: The top level menu items for workspaces and help have the same ID » Navigation's top level menu items for workspaces and help have the same ID when caching enabled
Issue summary: View changes

Updating IS and title.

kentr’s picture

kentr commented
StatusFileSize
new issue-3570505-10-empty-plugin-ids-exposed.png103.58 KB

Even with workspaces enabled, I'm not seeing Live

But for the Help item, the underlying probem appears to be that item.original_link.pluginId is empty.

This explains why Shortcuts has navigation-link---<digit>, even though it's not a duplicate.

Here's a screenshot with item.original_link.pluginId output on the page, between the plus signs (+).

For Help and Shortcuts, it's blank between the plus signs.

To me, the fix looks like getting item.original_link.pluginId to have values for those items.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

Even with workspaces enabled, I'm not seeing Live

Note you need to enable workspaces_ui module.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

To me, the fix looks like getting item.original_link.pluginId to have values for those items.

There's no reason that item.original_link.pluginId needs to be in the ID. The requirement is that the IDs are unique. Plus there will likely be multiple list-items with the same item.original_link.pluginId

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Active » Needs review

Tests updated to account for random numbers in IDs (and associated aria-controls attributes)

kentr’s picture

kentr commented

There's no reason that item.original_link.pluginId needs to be in the ID. The requirement is that the IDs are unique.

If that's the case, why not remove item.original_link.pluginId from the template?

To me, the inconsistency of some ID attributes with and some without the semantic part makes it look like a bug or like someone forgot something.

kentr’s picture

kentr commented

FYI, the pipeline has failures.

I didn't have time to look at the failures in depth

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

If that's the case, why not remove item.original_link.pluginId from the template?

Not sure why it was added. I guess I didn't remove it because it was out of scope? Maybe we could make a followup to remove it? TBH, I'm not sure it matters. But we could.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs work » Needs review

Pushed a fix to the selectors in the tests.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

If that's the case, why not remove item.original_link.pluginId from the template?

One more note on this is that the plugin ID is used within the tests (see below). We could move the plugin id to an attribute if needed. but just FYI, I think this would complicate things for minimal gain.

https://git.drupalcode.org/project/drupal/-/blob/9da49dfa40824c92b8670fd...

kentr’s picture

kentr commented
Issue summary: View changes

In case it matters, stable9 is also affected by this.

I set stable9 as the administration theme and saw the problem, so perhaps that means any admin theme created from stable9 per the current docs will be affected.

Starterkit uses stable9 as a base. Unsure if that will also cause problems.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Never know where stable9 lands but since this is the only other navigation-menu.html.twig in core maybe we should just update here too.

Rest of the changes LGTM and @mherchel probably good to RTBC yourself after the change.

kentr’s picture

kentr commented

I copied the change to stable9. When pipeline passes it should be ready for review.

kentr’s picture

kentr commented

Or, based on #21, RTBC when the pipeline passes.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs work » Reviewed & tested by the community

Pipeline passed. RTBC per #21 and #22

catch’s picture

catch
he/him
Primary language English
commented

Don't want to hold this up but on the other hand it really seems like a duplicate of #1852090: Cached render elements can have duplicate HTML IDs.

kentr’s picture

kentr commented

I'm experimenting with #1852090: Cached render elements can have duplicate HTML IDs.

kentr’s picture

kentr commented

I don't think this is really a duplicate of #1852090: Cached render elements can have duplicate HTML IDs.

Navigation is the only place in core that I can find currently using clean_unique_id.

There are other ways to generate a unique ID for cases like this. random() works, but concatenating the hierarchy and the link text would also probably work (like namespacing).

I think clean_unique_id just isn't ready for this type of use.

kentr’s picture

kentr commented

Or rather, the button text in this case.

  • catch committed b46b0c7a on 11.x 
    fix: #3570505 Navigation's top level menu items for workspaces and help...

  • catch committed b67852e9 on main 
    fix: #3570505 Navigation's top level menu items for workspaces and help...
catch’s picture

catch
he/him
Primary language English
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Thanks for looking at #1852090: Cached render elements can have duplicate HTML IDs. I still think we should be fixing that generically across core, but we can open a follow-up issue to standardize navigation on whatever ends up implemented in that issue.

Committed/pushed to main and 11.x, thanks!

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.

catch closed merge request !14732

kentr’s picture

kentr commented

I still think we should be fixing that generically across core

Same. And even though it's a global problem, it's acutely causing accessibility issues with forms.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.