From 6a192603d08a259644690ef17f0e807d8256c5e2 Mon Sep 17 00:00:00 2001
From: milanmajchrak <milan.majchrak@dataquest.sk>
Date: Thu, 10 Sep 2026 19:10:29 +0200
Subject: [PATCH] Port #1221 to dtq-dev-9-base: UFAL/Resolve duplicate HTML
 element IDs across pages

Card X-02a, part A of three. The v9 upgrade took these three templates wholesale from vanilla 9.3,
so the fork's hunks from 6628aaf493 were never applied and the pages emit duplicate DOM ids again.

ds-select renders one dropdown per sort option on a browse toolbar and one per pool task on
/mydspace, but hardcoded its three ids (dsSelectMenuLabel, dsSelectMenuButton, dsSelectDropdownMenu).
Every instance after the first therefore duplicated them, and each instance's aria-describedby /
aria-labelledby resolved to the *first* instance's elements - a screen reader announced the wrong
label for every dropdown but one. A module-level counter now gives each instance a uniqueId and the
three ids are suffixed with it.

Two adaptations beyond the source hunk, both because 9-base is not 7.x here:

* aria-describedby is bound as [attr.aria-describedby]="label ? 'dsSelectMenuLabel-' + uniqueId : null"
  rather than left hardcoded. The label span is inside @if (label), so without the null branch the
  button points at an element that does not exist whenever no label is set - a dangling aria
  reference. The source commit made the same choice; it is restated here because 9-base uses @if
  where 7.x used *ngIf.
* the pool-task row id is id="actions-{{ dso?.id }}" as in the source.

Tests added (the source commit ships none): a host rendering two ds-select instances asserts that
the document has no duplicate ids, that each button's aria-describedby resolves to a label inside its
own instance and to nothing else, and that the two button ids differ; a third case asserts
aria-describedby is null when no label is set; and the pool-task spec asserts the actions id is
row-specific rather than the constant "actions". Each was proven load-bearing with a negative
control - see the PR description.

Source: 6628aaf493 (dtq-dev PR #1221)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../shared/ds-select/ds-select.component.html | 10 +--
 .../ds-select/ds-select.component.spec.ts     | 69 +++++++++++++++++++
 .../shared/ds-select/ds-select.component.ts   |  9 +++
 ...-search-result-list-element.component.html |  2 +-
 ...arch-result-list-element.component.spec.ts | 11 ++-
 5 files changed, 94 insertions(+), 7 deletions(-)

diff --git a/src/app/shared/ds-select/ds-select.component.html b/src/app/shared/ds-select/ds-select.component.html
index c5fa369ceb5..8748fa28d9f 100644
--- a/src/app/shared/ds-select/ds-select.component.html
+++ b/src/app/shared/ds-select/ds-select.component.html
@@ -3,13 +3,13 @@
   <div ngbDropdown class="btn-group" (openChange)="toggled.emit($event)">
 
     @if (label) {
-      <span  id="dsSelectMenuLabel" class="input-group-text">
+      <span  id="dsSelectMenuLabel-{{ uniqueId }}" class="input-group-text">
         {{ label | translate }}
       </span>
     }
 
-    <button aria-describedby="dsSelectMenuLabel"
-      id="dsSelectMenuButton"
+    <button [attr.aria-describedby]="label ? 'dsSelectMenuLabel-' + uniqueId : null"
+      id="dsSelectMenuButton-{{ uniqueId }}"
       class="btn btn-outline-primary selection"
       (blur)="close.emit($event)"
       (click)="close.emit($event)"
@@ -20,8 +20,8 @@
 
     <div ngbDropdownMenu
       class="dropdown-menu"
-      id="dsSelectDropdownMenu"
-      aria-labelledby="dsSelectMenuButton">
+      id="dsSelectDropdownMenu-{{ uniqueId }}"
+      [attr.aria-labelledby]="'dsSelectMenuButton-' + uniqueId">
       <div>
         <ng-content select=".menu"></ng-content>
       </div>
diff --git a/src/app/shared/ds-select/ds-select.component.spec.ts b/src/app/shared/ds-select/ds-select.component.spec.ts
index 9bad8872ccd..5c8baea0e96 100644
--- a/src/app/shared/ds-select/ds-select.component.spec.ts
+++ b/src/app/shared/ds-select/ds-select.component.spec.ts
@@ -1,3 +1,4 @@
+import { Component } from '@angular/core';
 import {
   ComponentFixture,
   TestBed,
@@ -7,6 +8,23 @@ import { TranslateModule } from '@ngx-translate/core';
 
 import { DsSelectComponent } from './ds-select.component';
 
+/**
+ * Two ds-select instances on one page - the situation that produced duplicate DOM ids
+ * (a browse toolbar renders one per sort option, MyDSpace one per pool task).
+ */
+@Component({
+  selector: 'ds-test-host',
+  template: `
+    <ds-select label="first.label"><span class="selection">A</span></ds-select>
+    <ds-select label="second.label"><span class="selection">B</span></ds-select>
+  `,
+  imports: [
+    DsSelectComponent,
+  ],
+})
+class TestHostComponent {
+}
+
 describe('DsSelectComponent', () => {
   let component: DsSelectComponent;
   let fixture: ComponentFixture<DsSelectComponent>;
@@ -30,4 +48,55 @@ describe('DsSelectComponent', () => {
   it('should create', () => {
     expect(component).toBeTruthy();
   });
+
+  it('should not reference a label element when no label is set', () => {
+    const button: HTMLElement = fixture.nativeElement.querySelector('button.selection');
+
+    expect(button).toBeTruthy();
+    expect(button.getAttribute('aria-describedby')).toBeNull();
+    // the button still names its own menu
+    const menu: HTMLElement = fixture.nativeElement.querySelector('[ngbDropdownMenu]');
+    expect(menu.getAttribute('aria-labelledby')).toEqual(button.id);
+  });
+
+  describe('with two instances on the same page', () => {
+    let hostFixture: ComponentFixture<TestHostComponent>;
+    let hostElement: HTMLElement;
+
+    beforeEach(() => {
+      hostFixture = TestBed.createComponent(TestHostComponent);
+      hostFixture.detectChanges();
+      hostElement = hostFixture.nativeElement;
+    });
+
+    it('should not emit duplicate DOM ids', () => {
+      const ids: string[] = Array.from(hostElement.querySelectorAll('[id]')).map((element: Element) => element.id);
+
+      expect(ids.length).toBeGreaterThan(0);
+      expect(ids.length).toEqual(new Set(ids).size);
+    });
+
+    it('should resolve every aria reference inside its own instance', () => {
+      const selects: HTMLElement[] = Array.from(hostElement.querySelectorAll('ds-select'));
+      expect(selects.length).toEqual(2);
+
+      const buttonIds: string[] = [];
+      selects.forEach((select: HTMLElement) => {
+        const button: HTMLElement = select.querySelector('button.selection');
+        const menu: HTMLElement = select.querySelector('[ngbDropdownMenu]');
+        const describedBy: string = button.getAttribute('aria-describedby');
+
+        expect(describedBy).toBeTruthy();
+        // the label the button points at is this instance's own label ...
+        expect(select.querySelector('[id="' + describedBy + '"]')).toBeTruthy();
+        // ... and no other element in the document answers to that id
+        expect(hostElement.querySelectorAll('[id="' + describedBy + '"]').length).toEqual(1);
+        expect(menu.getAttribute('aria-labelledby')).toEqual(button.id);
+
+        buttonIds.push(button.id);
+      });
+
+      expect(buttonIds[0]).not.toEqual(buttonIds[1]);
+    });
+  });
 });
diff --git a/src/app/shared/ds-select/ds-select.component.ts b/src/app/shared/ds-select/ds-select.component.ts
index 95e076ac7de..3e1e202e1af 100644
--- a/src/app/shared/ds-select/ds-select.component.ts
+++ b/src/app/shared/ds-select/ds-select.component.ts
@@ -10,6 +10,8 @@ import { TranslateModule } from '@ngx-translate/core';
 
 import { BtnDisabledDirective } from '../btn-disabled.directive';
 
+let nextDsSelectId = 0;
+
 /**
  * Component which represent a DSpace dropdown selector.
  */
@@ -25,6 +27,13 @@ import { BtnDisabledDirective } from '../btn-disabled.directive';
 })
 export class DsSelectComponent {
 
+  /**
+   * Unique identifier for the component instance. Several ds-select instances are rendered on the
+   * same page (browse toolbars, MyDSpace), so the dropdown's DOM ids have to be per-instance or the
+   * document carries duplicate ids and every aria reference resolves to the first instance.
+   */
+  uniqueId = `ds-select-${nextDsSelectId++}`;
+
   /**
    * An optional label for the dropdown selector.
    */
diff --git a/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.html b/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.html
index 14c39dfa6c4..95e57589f27 100644
--- a/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.html
+++ b/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.html
@@ -19,7 +19,7 @@
   </ng-container>
   <div class="row">
     <div [ngClass]="showThumbnails ? 'offset-3 offset-md-2 col-9 col-md-10 ps-3' : ''">
-      <ds-pool-task-actions id="actions"
+      <ds-pool-task-actions id="actions-{{ dso?.id }}"
         [item]="item$.value"
         [object]="dso"
         [workflowitem]="workflowitem$.value"
diff --git a/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.spec.ts b/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.spec.ts
index 19f4c97b834..4621fdf228b 100644
--- a/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.spec.ts
+++ b/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.spec.ts
@@ -102,7 +102,7 @@ const environmentUseThumbs = {
 const rdItem = createSuccessfulRemoteDataObject(item);
 const workflowitem = Object.assign(new WorkflowItem(), { item: of(rdItem) });
 const rdWorkflowitem = createSuccessfulRemoteDataObject(workflowitem);
-mockResultObject.indexableObject = Object.assign(new PoolTask(), { workflowitem: of(rdWorkflowitem) });
+mockResultObject.indexableObject = Object.assign(new PoolTask(), { id: 'pool-task-1', workflowitem: of(rdWorkflowitem) });
 const linkService = getMockLinkService();
 const objectCacheServiceMock = jasmine.createSpyObj('ObjectCacheService', {
   remove: jasmine.createSpy('remove'),
@@ -171,4 +171,13 @@ describe('PoolSearchResultListElementComponent', () => {
     const thumbnail = fixture.debugElement.query(By.css('.offset-3'));
     expect(thumbnail).toBeTruthy();
   });
+
+  it('should give the pool task actions a row-specific id', () => {
+    const actions = fixture.debugElement.query(By.css('ds-pool-task-actions'));
+
+    expect(actions).toBeTruthy();
+    // /mydspace renders one of these per pool task, so a constant id duplicates across rows
+    expect(actions.nativeElement.id).not.toEqual('actions');
+    expect(actions.nativeElement.id).toContain(component.dso.id);
+  });
 });