From 2815a8b12b1ee65403c6e329a251a5eb216620f9 Mon Sep 17 00:00:00 2001
From: bram-atmire <bram@mire.be>
Date: Mon, 4 May 2026 11:20:55 +0200
Subject: [PATCH] Fix remaining ARIA issues from #5488

PR #5514 (also referencing #5488) makes the dso-edit-menu menubar role vanish
when the menu is empty/hidden. This commit addresses the four remaining ARIA
problems flagged in the issue and the supporting WAVE/Pa11y/HTMLCS findings:

1. auth-nav-menu (login dropdown trigger):
   The button declared aria-haspopup="menu" but actually opened a
   <div role="dialog" aria-modal="true">. Screen readers therefore announced
   "has popup, menu" and then revealed a dialog. Updated to
   aria-haspopup="dialog" so the announcement matches what opens. Also
   removed the redundant role="button" / tabindex="0" on a real <button>
   and added an explicit type="button".

2. expandable-navbar-section (top-level navbar section toggler):
   Replaced <a href="javascript:void(0);" role="menuitem"> with a real
   <button type="button" role="menuitem">. The navbar wrapper has
   role="menubar", so the menuitem nesting is valid; the issue was the
   href="javascript:void(0)" antipattern. Added a small SCSS reset so the
   button keeps the link-like appearance the previous anchor had.

3. context-help-toggle (header help button):
   This control is not nested in any role="menu" / role="menubar", so its
   role="menuitem" was an orphan flagged by axe (aria-required-parent).
   Replaced <a href="javascript:void(0);" role="menuitem"> with a plain
   <button type="button"> and dropped the orphan role.

4. dso-edit-menu-section (each section button under role="menubar"):
   When the parent menubar role is rendered (the case once #5514's
   visibility logic enables it), axe's aria-required-children rule still
   fires because the descendant <a>/<button> elements lack role="menuitem".
   Added role="menuitem" to each rendered control so the menubar pattern
   is structurally valid.

Spec updates accompany every template change.

References #5488
---
 .../context-help-toggle.component.html                 |  9 ++++-----
 .../context-help-toggle.component.scss                 |  7 +++++++
 .../context-help-toggle.component.spec.ts              |  9 ++++++++-
 .../expandable-navbar-section.component.html           |  4 ++--
 .../expandable-navbar-section.component.scss           | 10 ++++++++++
 .../expandable-navbar-section.component.spec.ts        |  9 +++++++++
 .../shared/auth-nav-menu/auth-nav-menu.component.html  |  5 ++---
 .../auth-nav-menu/auth-nav-menu.component.spec.ts      |  6 ++++++
 .../dso-edit-menu-section.component.html               |  5 +++--
 .../dso-edit-menu-section.component.spec.ts            |  5 +++++
 10 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/src/app/header/context-help-toggle/context-help-toggle.component.html b/src/app/header/context-help-toggle/context-help-toggle.component.html
index f815312739c..997ae80fb5c 100644
--- a/src/app/header/context-help-toggle/context-help-toggle.component.html
+++ b/src/app/header/context-help-toggle/context-help-toggle.component.html
@@ -1,12 +1,11 @@
 @if (buttonVisible$ | async) {
   <div>
-    <a href="javascript:void(0);"
-      role="menuitem"
+    <button type="button"
+      class="ds-context-help-toggle-button"
       (click)="onClick()"
       [attr.aria-label]="'nav.context-help-toggle' | translate"
-      [title]="'nav.context-help-toggle' | translate"
-      >
+      [title]="'nav.context-help-toggle' | translate">
       <i class="fas fa-lg fa-fw fa-question-circle ds-context-help-toggle"></i>
-    </a>
+    </button>
   </div>
 }
diff --git a/src/app/header/context-help-toggle/context-help-toggle.component.scss b/src/app/header/context-help-toggle/context-help-toggle.component.scss
index f3e5e01880a..f8cbd4047af 100644
--- a/src/app/header/context-help-toggle/context-help-toggle.component.scss
+++ b/src/app/header/context-help-toggle/context-help-toggle.component.scss
@@ -1,3 +1,10 @@
+.ds-context-help-toggle-button {
+  background: none;
+  border: 0;
+  padding: 0;
+  cursor: pointer;
+}
+
 .ds-context-help-toggle {
   color: var(--ds-header-icon-color);
   background-color: var(--ds-header-bg);
diff --git a/src/app/header/context-help-toggle/context-help-toggle.component.spec.ts b/src/app/header/context-help-toggle/context-help-toggle.component.spec.ts
index 5172cfd9913..45995afcd87 100644
--- a/src/app/header/context-help-toggle/context-help-toggle.component.spec.ts
+++ b/src/app/header/context-help-toggle/context-help-toggle.component.spec.ts
@@ -56,10 +56,17 @@ describe('ContextHelpToggleComponent', () => {
     });
 
     it('clicking the button should toggle context help icon visibility', fakeAsync(() => {
-      fixture.debugElement.query(By.css('a')).nativeElement.click();
+      fixture.debugElement.query(By.css('button')).nativeElement.click();
       tick();
       expect(contextHelpService.toggleIcons).toHaveBeenCalled();
     }));
+
+    it('the toggle should be a real <button> without an orphan role="menuitem"', () => {
+      const toggle = fixture.debugElement.query(By.css('button')).nativeElement;
+      expect(toggle.tagName).toBe('BUTTON');
+      expect(toggle.getAttribute('role')).toBeNull();
+      expect(toggle.getAttribute('type')).toBe('button');
+    });
   });
 
 });
diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html
index 05e9fde9816..64c8e9c779e 100644
--- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html
+++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html
@@ -4,7 +4,7 @@
     (mouseenter)="onMouseEnter($event)"
     (mouseleave)="onMouseLeave($event)"
     data-test="navbar-section-wrapper">
-    <a href="javascript:void(0);" routerLinkActive="active"
+    <button type="button"
       role="menuitem"
       (keyup.enter)="toggleSection($event)"
       (keyup.space)="toggleSection($event)"
@@ -21,7 +21,7 @@
         *ngComponentOutlet="(sectionMap$ | async).get(section.id).component; injector: (sectionMap$ | async).get(section.id).injector;"></ng-container>
       </span>
       <i class="fas fa-caret-down fa-xs toggle-menu-icon" aria-hidden="true"></i>
-    </a>
+    </button>
     @if ((active$ | async).valueOf() === true) {
       <div (click)="deactivateSection($event)"
         [id]="expandableNavbarSectionId()"
diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.scss b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.scss
index ab0934e9024..80856de13bc 100644
--- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.scss
+++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.scss
@@ -3,6 +3,16 @@
     position: relative; // align dropdown menu with respect to this element
   }
 
+  // The toggler is a <button> so user agents do not apply native button chrome
+  button.ds-menu-toggler-wrapper {
+    background: none;
+    border: 0;
+    padding: 0;
+    color: inherit;
+    font: inherit;
+    cursor: pointer;
+  }
+
   .dropdown-menu {
     overflow: hidden;
     min-width: 100%;
diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts
index fb0e66d9e82..5535b3908bf 100644
--- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts
+++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts
@@ -57,6 +57,15 @@ describe('ExpandableNavbarSectionComponent', () => {
       fixture.detectChanges();
     });
 
+    describe('toggler element', () => {
+      it('should be a real <button> rather than an anchor with href="javascript:void(0)"', () => {
+        const toggler = fixture.debugElement.query(By.css('[data-test="navbar-section-toggler"]')).nativeElement;
+        expect(toggler.tagName).toBe('BUTTON');
+        expect(toggler.getAttribute('type')).toBe('button');
+        expect(toggler.getAttribute('role')).toBe('menuitem');
+      });
+    });
+
     describe('when the mouse enters the section header (while inactive)', () => {
       beforeEach(() => {
         spyOn(component, 'onMouseEnter').and.callThrough();
diff --git a/src/app/shared/auth-nav-menu/auth-nav-menu.component.html b/src/app/shared/auth-nav-menu/auth-nav-menu.component.html
index 5f5a89db4db..28e19bfeb6b 100644
--- a/src/app/shared/auth-nav-menu/auth-nav-menu.component.html
+++ b/src/app/shared/auth-nav-menu/auth-nav-menu.component.html
@@ -9,9 +9,8 @@
         <div ngbDropdown #loginDrop="ngbDropdown" display="dynamic" placement="bottom-right" class="d-inline-block" @fadeInOut>
           <button class="dropdownLogin btn btn-link px-0" [attr.aria-label]="'nav.login' |translate"
                   (click)="$event.preventDefault()" [attr.data-test]="'login-menu' | dsBrowserOnly"
-                  role="button"
-                  tabindex="0"
-                  aria-haspopup="menu"
+                  type="button"
+                  aria-haspopup="dialog"
                   aria-controls="loginDropdownMenu"
                   [attr.aria-expanded]="loginDrop.isOpen()"
                   ngbDropdownToggle>
diff --git a/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts b/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts
index fea2e03444b..9fb5edbd307 100644
--- a/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts
+++ b/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts
@@ -234,6 +234,12 @@ describe('AuthNavMenuComponent', () => {
           const loginDropdownMenu = deNavMenuItem.query(By.css('div#loginDropdownMenu'));
           expect(loginDropdownMenu.nativeElement).toBeDefined();
         });
+
+        it('the login toggle should declare aria-haspopup="dialog" so it matches the role of the popup it controls', () => {
+          const loginToggle = deNavMenuItem.query(By.css('button.dropdownLogin'));
+          expect(loginToggle.nativeElement.getAttribute('aria-haspopup')).toBe('dialog');
+          expect(loginToggle.nativeElement.getAttribute('aria-controls')).toBe('loginDropdownMenu');
+        });
       });
 
       describe('when user is authenticated', () => {
diff --git a/src/app/shared/dso-page/dso-edit-menu/dso-edit-menu-section/dso-edit-menu-section.component.html b/src/app/shared/dso-page/dso-edit-menu/dso-edit-menu-section/dso-edit-menu-section.component.html
index b2c8ca84ce9..425ff2f90fa 100644
--- a/src/app/shared/dso-page/dso-edit-menu/dso-edit-menu-section/dso-edit-menu-section.component.html
+++ b/src/app/shared/dso-page/dso-edit-menu/dso-edit-menu-section/dso-edit-menu-section.component.html
@@ -3,13 +3,14 @@
     [ngbTooltip]="itemModel.text | translate">
     @if (!section.model.disabled) {
       <a class="btn btn-dark btn-sm"
+        role="menuitem"
         [routerLink]="itemModel.link">
         <i class="fas fa-{{section.icon}} fa-fw" aria-hidden="true"></i>
         <span class="sr-only">{{itemModel.text | translate}}</span>
       </a>
     }
     @if (section.model.disabled) {
-      <button class="btn btn-dark btn-sm" [dsBtnDisabled]="true">
+      <button class="btn btn-dark btn-sm" role="menuitem" [dsBtnDisabled]="true">
         <i class="fas fa-{{section.icon}} fa-fw" aria-hidden="true"></i>
         <span class="sr-only">{{itemModel.text | translate}}</span>
       </button>
@@ -20,7 +21,7 @@
 @if (canActivate) {
   <div class="dso-button-menu mb-1"
     [ngbTooltip]="itemModel.text | translate">
-    <button class="btn btn-dark btn-sm" [dsBtnDisabled]="section.model.disabled"
+    <button class="btn btn-dark btn-sm" role="menuitem" [dsBtnDisabled]="section.model.disabled"
       (click)="activate($event)">
       <i class="fas fa-{{section.icon}} fa-fw" aria-hidden="true"></i>
       <span class="sr-only">{{itemModel.text | translate}}</span>
diff --git a/src/app/shared/dso-page/dso-edit-menu/dso-edit-menu-section/dso-edit-menu-section.component.spec.ts b/src/app/shared/dso-page/dso-edit-menu/dso-edit-menu-section/dso-edit-menu-section.component.spec.ts
index 4e5ebf67224..200bbc626c5 100644
--- a/src/app/shared/dso-page/dso-edit-menu/dso-edit-menu-section/dso-edit-menu-section.component.spec.ts
+++ b/src/app/shared/dso-page/dso-edit-menu/dso-edit-menu-section/dso-edit-menu-section.component.spec.ts
@@ -109,6 +109,11 @@ describe('DsoEditMenuSectionComponent', () => {
       expect(button.nativeElement.innerHTML).toContain('fa-' + iconString);
     });
 
+    it('should declare role="menuitem" so the parent menubar satisfies aria-required-children', () => {
+      const button = fixture.debugElement.query(By.css('.btn-dark'));
+      expect(button.nativeElement.getAttribute('role')).toBe('menuitem');
+    });
+
     describe('when the section model in a disabled link or text', () => {
       it('should show just the button', () => {
         const textButton = fixture.debugElement.query(By.css('div a'));