Duplicate ID tags on menu items [#2939629]

If I create menu items with the same name (a link in the header called Home, and one in the footer called Home) I end up with duplicate id tags on the li elements in the menu. This is because the mlid value is automatically generated based on the text in the link. This occurs even with different menu blocks. I've seen other menus where a unique 4 number id is added to each of the mlid values so you would end up with a unique id, something like id="menu-mlid-1023". Is this the way this is supposed to work, and is it something that can be easily patched?

Thanks.

Comments

Comment #1

25 January 2018 at 23:26

jmoruzi created an issue. See original summary.

Comment #2

jmoruzi

Brantford, ON

CreditAttribution: jmoruzi commented 25 January 2018 at 23:27

Version:

8.x-1.0

» 8.x-2.0

Comment #3

Jeff Burnz CreditAttribution: Jeff Burnz commented 28 January 2018 at 20:52

Did you see that in a Drupal 8 theme, or a Drupal 7 theme? I've struggled to get something like an actual mlid out of D8, so I'd be very interested to see how someone else might be doing that.

Comment #4

Jeff Burnz CreditAttribution: Jeff Burnz commented 28 January 2018 at 20:56

What I could do is add the menu name as a prefix - so for example if you had a Main menu and a Footer menu and two links for "Home", they might look like this

id="menu-main__home"
id="menu-footer__home"

Thats a pretty big change for existing themes, which is my only real concern.

Comment #5

jmoruzi

Brantford, ON

CreditAttribution: jmoruzi commented 1 February 2018 at 17:37

On one of my sites (Omega for D7) I have a Superfish menu in the header which gives menu ids like this: id="menu-218-1".

In the footer is a menu block which has the same menu (not Superfish) that does not have an id but has the class: class="menu-mlid-218"

So it looks like it's the Superfish module that generating the id because I have a few D8 sites that use the Basic theme and there are no ids on the menu items, only classes. Does there need to be an id or could you just have a class on the menu item?

Comment #6

Jeff Burnz CreditAttribution: Jeff Burnz commented 2 February 2018 at 21:57

You don't need an ID, there are always ways of targeting a menu item using CSS, however it's probably the most efficient and easiest way.

EDIT - I should have been more clear, I meant you don't need an ID for styling, however it is used for accessibility - see below.

Comment #7

jmoruzi

Brantford, ON

CreditAttribution: jmoruzi commented 2 February 2018 at 14:16

Would removing the id affect accessibility? In the menu.html.twig file, line 64:

{# We set an id on list items to provide context for aria attributes in responsive menu toggle buttons. #}
      <li{{ item.attributes.addClass(item_classes).setAttribute('id', 'mlid-' ~ item.title|render|clean_class)|without('role') }}>

I don't see how the menu id attribute is used, or would have any effect on usability.

Comment #8

Jeff Burnz CreditAttribution: Jeff Burnz commented 2 February 2018 at 20:34

It is used for accessibility - to bind the click handle in responsive menus to the menu item.

Comment #9

Jeff Burnz CreditAttribution: Jeff Burnz commented 2 February 2018 at 22:07

I'm favouring this pattern:

<li{{ item.attributes.addClass(item_classes)|without('role') }} id="{{ 'menu-name--' ~ menu_name ~ '__' ~ item.title|render|clean_id }}">

Which results in output like this:

<li class="menu__item menu__item-title--home" id="menu-name--main__home">

While the menu classes are:

<ul class="menu odd menu-level-1 menu-name--main">

The id is therefor a modifier pattern on the unique menu name class (in the example it's menu-name--main, which kind of makes sense given it's current usage (for binding click handles).

I doubt that many themes are using the ID for styling, and if they are it will be a very minor update to fix any issues. The upside is we fix the duplicate ID issue and overall it's easier code to modify (by not using setAttribute).

Comment #10

2 February 2018 at 22:12

Jeff Burnz committed c70534d on 8.x-3.x

Issue #2939629: Duplicate ID tags on menu items

Comment #11

Jeff Burnz CreditAttribution: Jeff Burnz commented 2 February 2018 at 22:12

Status:

Active

» Fixed

Comment #12

jmoruzi

Brantford, ON

CreditAttribution: jmoruzi commented 5 February 2018 at 20:13

Thanks Jeff, much appreciated

Comment #13

jmoruzi

Brantford, ON

CreditAttribution: jmoruzi commented 8 February 2018 at 02:37

Apologies Jeff, perhaps I was not clear enough. The problem still exists. The issue is not just with the anchor text, the problem occurs when placing a menu in 2 places using the block system. If use a menu in 2 different places, the menu name is still the same. I have a main menu in the header, here's the actual code generated on the page:

<nav role="navigation" aria-labelledby="block-mainnavigation-menu" id="block-mainnavigation" class="contextual-region l-bl block block-menu block-config-provider--system block-plugin-id--system-menu-block-main">
  <div class="block__inner block-menu__inner">

    
    <h2 class="visually-hidden block__title block-menu__title" id="block-mainnavigation-menu"><span>Services</span></h2>
    <div data-contextual-id="block:block=mainnavigation:langcode=en|menu:menu=main:langcode=en"></div><div class="block__content block-menu__content">
    <ul class="menu odd menu-level-1 menu-name--main">      
                          
      
            <li class="menu__item menu__item-title--controls--automation" id="menu-name--main__controls-automation">

        <span class="menu__link--wrapper">
          <a href="/controls-automation" class="menu__link" data-drupal-link-system-path="node/13">Controls &amp; Automation</a>
        </span>

        
      </li>      
                          
      
            <li class="menu__item menu__item-title--mes-mom" id="menu-name--main__mesmom">

        <span class="menu__link--wrapper">
          <a href="/mes-mom" class="menu__link" data-drupal-link-system-path="node/12">MES/MOM</a>
        </span>

        
      </li>      
                          
      
            <li class="menu__item menu__item-title--iot--smart-manufacturing" id="menu-name--main__iot-smart-manufacturing">

        <span class="menu__link--wrapper">
          <a href="/iot-smart-manufacturing" class="menu__link" data-drupal-link-system-path="node/11">IoT &amp; Smart Manufacturing</a>
        </span>

        
      </li>      
                          
      
            <li class="menu__item menu__item-title--manufacturing-consulting" id="menu-name--main__manufacturing-consulting">

        <span class="menu__link--wrapper">
          <a href="/manufacturing-consulting" class="menu__link" data-drupal-link-system-path="node/9">Manufacturing Consulting</a>
        </span>

        
      </li></ul>
  

</div></div>
</nav>

I've placed a block in the footer with the same menu, here's that code:

<nav role="navigation" aria-labelledby="block-mainnavigation-2-menu" id="block-mainnavigation-2" class="contextual-region l-bl block block-menu block-config-provider--system block-plugin-id--system-menu-block-main has-title">
  <div class="block__inner block-menu__inner">

    
    <h2 class="block__title block-menu__title" id="block-mainnavigation-2-menu"><span>What We Do</span></h2>
    <div data-contextual-id="block:block=mainnavigation_2:langcode=en|menu:menu=main:langcode=en"></div><div class="block__content block-menu__content">
    <ul class="menu odd menu-level-1 menu-name--main">      
                          
      
            <li class="menu__item menu__item-title--controls--automation" id="menu-name--main__controls-automation">

        <span class="menu__link--wrapper">
          <a href="/controls-automation" class="menu__link" data-drupal-link-system-path="node/13">Controls &amp; Automation</a>
        </span>

        
      </li>      
                          
      
            <li class="menu__item menu__item-title--mes-mom" id="menu-name--main__mesmom">

        <span class="menu__link--wrapper">
          <a href="/mes-mom" class="menu__link" data-drupal-link-system-path="node/12">MES/MOM</a>
        </span>

        
      </li>      
                          
      
            <li class="menu__item menu__item-title--iot--smart-manufacturing" id="menu-name--main__iot-smart-manufacturing">

        <span class="menu__link--wrapper">
          <a href="/iot-smart-manufacturing" class="menu__link" data-drupal-link-system-path="node/11">IoT &amp; Smart Manufacturing</a>
        </span>

        
      </li>      
                          
      
            <li class="menu__item menu__item-title--manufacturing-consulting" id="menu-name--main__manufacturing-consulting">

        <span class="menu__link--wrapper">
          <a href="/manufacturing-consulting" class="menu__link" data-drupal-link-system-path="node/9">Manufacturing Consulting</a>
        </span>

        
      </li></ul>
  

</div></div>
</nav>

The nav id is unique, but because the menu name is the same the li ids are still the same.

Comment #14

jmoruzi

Brantford, ON

CreditAttribution: jmoruzi commented 7 February 2018 at 16:21

Status:

Fixed

» Needs work

Comment #15

Jeff Burnz CreditAttribution: Jeff Burnz commented 7 February 2018 at 20:56

OK, yes good point, I'll have to see if I can pass something from the block - the block instance is different (so the unique block ID is coming from the block system, but calling the same menu etc).

Comment #16

Jeff Burnz CreditAttribution: Jeff Burnz commented 8 February 2018 at 00:58

Status:

Needs work

» Postponed

This might have to fall into the "good enough" category, basically because I have run out of time to figure out how this could work - the menu nor menu items have any such thing that makes them unique other than the block plugin id, and getting that into the menu template seems very problematic (suggestions more than welcome).

Styling wise you can style them, by using the block ID, however I am very reluctant to just remove the ID's due to a duplicate ID warning etc, because they do, in most instances, provide context for the click handles for screen reader users.

I can accept this a bug, because technically it is, but how to fix it seems quite problematic, I might try asking on Stack Exchange to get someone more familiar with the menu system especially SystemMenuBlock.php and the final build process.

Comment #17

jmoruzi

Brantford, ON

CreditAttribution: jmoruzi commented 8 February 2018 at 02:42

OK, thanks. I'll need to see if I can come up with a workaround because I can't just remove the the ids for the same reason. This site needs to meet accessibility requirements.