fix(client): scope the ARIA pass to each grid and drive it from the render hooks
Build / Build-and-ng-test (pull_request) Canceled after 0s
Build / Build-and-test-development (pull_request) Canceled after 0s
Lighthouse Checks / lighthouse (pull_request) Canceled after 0s

The editor and viewer each ran fixAriaAccessibility() over the whole
document on every render, on a 3-second interval and from a
MutationObserver on document.body, and each call scheduled four further
copies of itself. The sweep removes the role/aria attributes that its own
observer watches, so it re-armed itself.

Measured with axe-core against Handsontable 18: the default markup fails
aria-required-children (critical) because the treegrid wrapper has no row
children, and disabling the tags with ariaTags:false only trades that for
empty-table-header while leaving the placeholder semantics missing. The
existing attribute sweep is therefore the right end state - it just has to
run against one grid's root, not the document.

Both components now call one shared helper with their own
hotInstance.rootElement, and the interval, the body observer and the four
deferred re-runs are gone; the afterRender/afterChange hooks and the init
calls are the only triggers.
This commit is contained in:
dc committed 2026-11-01 22:41:23 +00:00
1 parent 663088ad16
commit bf1e7d6b15
4 files changed
+191 -315

No files matched your search

+9 -157
View File
@@ -15,6 +15,7 @@ import { ActivatedRoute, Router } from '@angular/router'
import Handsontable, { CellRange } from 'handsontable' import Handsontable, { CellRange } from 'handsontable'
import { Subject, Subscription } from 'rxjs' import { Subject, Subscription } from 'rxjs'
import { sanitiseForSas } from '../shared/utils/sanitise' import { sanitiseForSas } from '../shared/utils/sanitise'
import { fixGridAria } from '../shared/utils/grid-aria'
import { SasStoreService } from '../services/sas-store.service' import { SasStoreService } from '../services/sas-store.service'
type AOA = any[][] type AOA = any[][]
@@ -714,8 +715,6 @@ export class EditorComponent
public licenceState = this.licenceService.licenceState public licenceState = this.licenceService.licenceState
private ariaObserver: MutationObserver | undefined
private ariaCheckInterval: any | undefined
private gridResizeObserver: ResizeObserver | undefined private gridResizeObserver: ResizeObserver | undefined
private pasteListener: ((event: ClipboardEvent) => void) | undefined private pasteListener: ((event: ClipboardEvent) => void) | undefined
@@ -3759,18 +3758,6 @@ export class EditorComponent
ngOnDestroy() { ngOnDestroy() {
this._queryParams?.unsubscribe() this._queryParams?.unsubscribe()
// Clean up the MutationObserver
if (this.ariaObserver) {
this.ariaObserver.disconnect()
this.ariaObserver = undefined
}
// Clean up the interval
if (this.ariaCheckInterval) {
clearInterval(this.ariaCheckInterval)
this.ariaCheckInterval = undefined
}
// Stop observing the grid container for resizes // Stop observing the grid container for resizes
if (this.gridResizeObserver) { if (this.gridResizeObserver) {
this.gridResizeObserver.disconnect() this.gridResizeObserver.disconnect()
@@ -3820,150 +3807,15 @@ export class EditorComponent
} }
/** /**
* Fixes ARIA accessibility issues in the Handsontable component * Corrects the ARIA markup Handsontable puts on its own grid wrapper - see
* This addresses the accessibility report issues with treegrid and presentation roles * shared/utils/grid-aria.ts for what is corrected and why. Scoped to this
* grid so it does not touch any other grid or component on the page.
*/ */
private fixAriaAccessibility() { private fixAriaAccessibility(): void {
// Use a more aggressive approach to find and fix all ARIA issues fixGridAria(
const fixAriaIssues = () => { this.hotInstance?.rootElement ??
// Specifically target Handsontable wrapper elements that are causing issues this.hotTableComponent?.hotInstance?.rootElement
const hotWrappers = document.querySelectorAll( )
'.ht-wrapper, .wtHolder, [id^="ht_"]'
)
hotWrappers.forEach((wrapper) => {
// Remove problematic ARIA attributes from Handsontable wrappers
wrapper.removeAttribute('role')
wrapper.removeAttribute('aria-rowcount')
wrapper.removeAttribute('aria-colcount')
wrapper.removeAttribute('aria-multiselectable')
})
// Find all elements with problematic ARIA roles in the entire document
const allTreegridElements = document.querySelectorAll('[role="treegrid"]')
const allPresentationElements = document.querySelectorAll(
'[role="presentation"]'
)
// Fix treegrid role issues - remove them completely as they're causing problems
allTreegridElements.forEach((element) => {
element.removeAttribute('role')
element.removeAttribute('aria-rowcount')
element.removeAttribute('aria-colcount')
element.removeAttribute('aria-multiselectable')
})
// Fix presentation role issues - remove them if they contain interactive elements
allPresentationElements.forEach((element) => {
const hasInteractiveChildren =
element.querySelectorAll(
'button, input, select, textarea, [tabindex], [onclick], [contenteditable]'
).length > 0
if (hasInteractiveChildren) {
element.removeAttribute('role')
}
})
// Also fix any elements with aria-rowcount="-1" which is problematic
const negativeRowCountElements = document.querySelectorAll(
'[aria-rowcount="-1"]'
)
negativeRowCountElements.forEach((element) => {
element.removeAttribute('aria-rowcount')
})
// Ensure proper table structure
const tableElements = document.querySelectorAll('table')
tableElements.forEach((table) => {
if (!table.getAttribute('role')) {
table.setAttribute('role', 'table')
}
// Ensure table headers have proper scope
const headerCells = table.querySelectorAll('th')
headerCells.forEach((th) => {
if (!th.getAttribute('scope')) {
th.setAttribute('scope', 'col')
}
})
})
// Add proper ARIA labels to interactive elements
const interactiveElements = document.querySelectorAll(
'button, input, select, textarea, [contenteditable]'
)
interactiveElements.forEach((element) => {
if (
!element.getAttribute('aria-label') &&
!element.getAttribute('aria-labelledby')
) {
const textContent = element.textContent?.trim()
if (textContent) {
element.setAttribute('aria-label', textContent)
}
}
})
}
// Run the fix immediately
fixAriaIssues()
// Run it again after a short delay to catch any dynamically created elements
setTimeout(fixAriaIssues, 100)
setTimeout(fixAriaIssues, 500)
setTimeout(fixAriaIssues, 1000)
setTimeout(fixAriaIssues, 2000)
// Set up a periodic check to ensure accessibility fixes are maintained
if (!this.ariaCheckInterval) {
this.ariaCheckInterval = setInterval(fixAriaIssues, 3000)
}
// Set up a MutationObserver to continuously monitor for new problematic elements
if (!this.ariaObserver) {
this.ariaObserver = new MutationObserver((mutations) => {
let shouldFix = false
mutations.forEach((mutation) => {
if (
mutation.type === 'attributes' &&
(mutation.attributeName === 'role' ||
mutation.attributeName === 'aria-rowcount')
) {
shouldFix = true
}
if (mutation.type === 'childList') {
mutation.addedNodes.forEach((node) => {
if (node.nodeType === Node.ELEMENT_NODE) {
const element = node as Element
if (
element.hasAttribute('role') ||
element.hasAttribute('aria-rowcount')
) {
shouldFix = true
}
}
})
}
})
if (shouldFix) {
setTimeout(fixAriaIssues, 50)
}
})
// Start observing the entire document for changes
this.ariaObserver.observe(document.body, {
childList: true,
subtree: true,
attributes: true,
attributeFilter: [
'role',
'aria-rowcount',
'aria-colcount',
'aria-multiselectable'
]
})
}
} }
initSetup(response: EditorsGetDataServiceResponse, attempt = 0) { initSetup(response: EditorsGetDataServiceResponse, attempt = 0) {
@@ -0,0 +1,99 @@
import { fixGridAria } from './grid-aria'
const buildGrid = (id: string): HTMLElement => {
const container = document.createElement('div')
container.id = `container_${id}`
container.innerHTML = `
<div class="ht-wrapper handsontable" id="ht_${id}" role="treegrid"
aria-rowcount="-1" aria-colcount="3" aria-multiselectable="true">
<div class="wtHolder" role="presentation">
<table class="htCore">
<thead><tr><th>Class</th></tr></thead>
<tbody><tr><td><button>Edit record</button></td></tr></tbody>
</table>
</div>
</div>`
return container
}
describe('fixGridAria', () => {
it('strips the treegrid role and the row/column counts from the grid wrapper', () => {
const container = buildGrid('a')
const wrapper = container.querySelector('.ht-wrapper') as HTMLElement
fixGridAria(container)
expect(wrapper.getAttribute('role')).toBeNull()
expect(wrapper.getAttribute('aria-rowcount')).toBeNull()
expect(wrapper.getAttribute('aria-colcount')).toBeNull()
expect(wrapper.getAttribute('aria-multiselectable')).toBeNull()
})
it('drops a presentation role only when the element holds focusable content', () => {
const container = buildGrid('b')
const interactive = container.querySelector('.wtHolder') as HTMLElement
const empty = document.createElement('div')
empty.setAttribute('role', 'presentation')
empty.innerHTML = '<span>overlay</span>'
container.appendChild(empty)
fixGridAria(container)
expect(interactive.getAttribute('role')).toBeNull()
expect(empty.getAttribute('role')).toBe('presentation')
})
it('gives the table an explicit table role and its headers a column scope', () => {
const container = buildGrid('c')
fixGridAria(container)
expect(container.querySelector('table')?.getAttribute('role')).toBe('table')
expect(container.querySelector('th')?.getAttribute('scope')).toBe('col')
})
it('labels interactive elements from their text, keeping an existing label', () => {
const container = buildGrid('d')
const labelled = document.createElement('button')
labelled.setAttribute('aria-label', 'Existing')
labelled.textContent = 'Overwrite me'
container.appendChild(labelled)
fixGridAria(container)
expect(
container.querySelector('table button')?.getAttribute('aria-label')
).toBe('Edit record')
expect(labelled.getAttribute('aria-label')).toBe('Existing')
})
it('leaves markup outside the grid container untouched', () => {
const container = buildGrid('e')
const other = buildGrid('f')
document.body.appendChild(container)
document.body.appendChild(other)
const otherWrapper = other.querySelector('.ht-wrapper') as HTMLElement
fixGridAria(container)
expect(otherWrapper.getAttribute('role')).toBe('treegrid')
expect(otherWrapper.getAttribute('aria-rowcount')).toBe('-1')
container.remove()
other.remove()
})
it('is idempotent', () => {
const container = buildGrid('g')
fixGridAria(container)
const first = container.innerHTML
fixGridAria(container)
expect(container.innerHTML).toBe(first)
})
it('accepts a missing grid, which is the case before Handsontable has mounted', () => {
expect(() => fixGridAria(undefined)).not.toThrow()
})
})
+77
View File
@@ -0,0 +1,77 @@
/** Elements that count as focusable/interactive for the presentation-role check. */
const INTERACTIVE =
'button, input, select, textarea, [tabindex], [onclick], [contenteditable]'
/** Interactive elements that need an accessible name. */
const NAMED = 'button, input, select, textarea, [contenteditable]'
/**
* Handsontable 18 marks its grid wrapper `role="treegrid"` with
* `aria-rowcount="-1"`, and marks the scroll holder `role="presentation"`.
* Neither survives an axe audit:
*
* - a treegrid must own rows (axe: `aria-required-children`, critical), but the
* wrapper's direct children are overlay divs, so the role is a lie;
* - `aria-rowcount="-1"` tells assistive tech the total row count is unknown
* when the table has already rendered it;
* - `role="presentation"` on the scroll holder that contains the focusable
* cells hides those cells from assistive tech.
*
* Measured on Handsontable 18 with axe-core: the default markup fails the first
* two rules; disabling the tags (`ariaTags: false`) only trades the critical
* failure for an `empty-table-header` one and leaves the scroll region
* unfocusable. So the attributes are corrected here instead - the wrapper is
* demoted to a plain container and the table gets an explicit `role="table"`
* with scoped headers.
*
* The pass is scoped to one grid's container, not the document: each grid owns
* its own markup, and a document-wide sweep would fight every other grid on the
* page (and every other component's DOM) on each render. It is idempotent, so
* calling it after every render and every data change is safe.
*/
const collect = (root: HTMLElement, selector: string): Element[] => {
const found = Array.from(root.querySelectorAll(selector))
return root.matches(selector) ? [root, ...found] : found
}
export const fixGridAria = (root: HTMLElement | null | undefined): void => {
if (!root) return
for (const element of collect(
root,
'.ht-wrapper, .wtHolder, [id^="ht_"], [role="treegrid"]'
)) {
element.removeAttribute('role')
element.removeAttribute('aria-rowcount')
element.removeAttribute('aria-colcount')
element.removeAttribute('aria-multiselectable')
}
for (const element of collect(root, '[role="presentation"]')) {
if (element.querySelector(INTERACTIVE)) element.removeAttribute('role')
}
for (const element of collect(root, '[aria-rowcount="-1"]')) {
element.removeAttribute('aria-rowcount')
}
for (const table of collect(root, 'table')) {
if (!table.getAttribute('role')) table.setAttribute('role', 'table')
for (const headerCell of Array.from(table.querySelectorAll('th'))) {
if (!headerCell.getAttribute('scope')) {
headerCell.setAttribute('scope', 'col')
}
}
}
for (const element of collect(root, NAMED)) {
if (
!element.getAttribute('aria-label') &&
!element.getAttribute('aria-labelledby')
) {
const textContent = element.textContent?.trim()
if (textContent) element.setAttribute('aria-label', textContent)
}
}
}
+6 -158
View File
@@ -48,6 +48,7 @@ import { Col } from '../shared/dc-validator/models/col.model'
import { parseLabelsParam } from '../shared/utils/parse-labels-param' import { parseLabelsParam } from '../shared/utils/parse-labels-param'
import { getDisplayColHeaders } from '../shared/utils/display-col-headers' import { getDisplayColHeaders } from '../shared/utils/display-col-headers'
import { buildColInfoHtml } from '../shared/utils/col-info-html' import { buildColInfoHtml } from '../shared/utils/col-info-html'
import { fixGridAria } from '../shared/utils/grid-aria'
@Component({ @Component({
selector: 'app-viewer', selector: 'app-viewer',
@@ -174,9 +175,6 @@ export class ViewerComponent
public licenceState = this.licenceService.licenceState public licenceState = this.licenceService.licenceState
public Infinity = Infinity public Infinity = Infinity
private ariaObserver: MutationObserver | undefined
private ariaCheckInterval: any | undefined
public hotTable: HotTableInterface = { public hotTable: HotTableInterface = {
data: [], data: [],
colHeaders: [], colHeaders: [],
@@ -1525,165 +1523,15 @@ export class ViewerComponent
// Safely destroy Handsontable instance // Safely destroy Handsontable instance
this.cleanupHotInstance() this.cleanupHotInstance()
// Clean up ARIA accessibility observers
if (this.ariaObserver) {
this.ariaObserver.disconnect()
this.ariaObserver = undefined
}
// Clear ARIA check intervals
if (this.ariaCheckInterval) {
clearInterval(this.ariaCheckInterval)
this.ariaCheckInterval = undefined
}
} }
/** /**
* Fixes ARIA accessibility issues in the Handsontable component * Corrects the ARIA markup Handsontable puts on its own grid wrapper - see
* This addresses the accessibility report issues with treegrid and presentation roles * shared/utils/grid-aria.ts for what is corrected and why. Scoped to this
* grid so it does not touch any other grid or component on the page.
*/ */
private fixAriaAccessibility() { private fixAriaAccessibility(): void {
// Use a more aggressive approach to find and fix all ARIA issues fixGridAria(this.hotInstance?.rootElement)
const fixAriaIssues = () => {
// Specifically target Handsontable wrapper elements that are causing issues
const hotWrappers = document.querySelectorAll(
'.ht-wrapper, .wtHolder, [id^="ht_"]'
)
hotWrappers.forEach((wrapper) => {
// Remove problematic ARIA attributes from Handsontable wrappers
wrapper.removeAttribute('role')
wrapper.removeAttribute('aria-rowcount')
wrapper.removeAttribute('aria-colcount')
wrapper.removeAttribute('aria-multiselectable')
})
// Find all elements with problematic ARIA roles in the entire document
const allTreegridElements = document.querySelectorAll('[role="treegrid"]')
const allPresentationElements = document.querySelectorAll(
'[role="presentation"]'
)
// Fix treegrid role issues - remove them completely as they're causing problems
allTreegridElements.forEach((element) => {
element.removeAttribute('role')
element.removeAttribute('aria-rowcount')
element.removeAttribute('aria-colcount')
element.removeAttribute('aria-multiselectable')
})
// Fix presentation role issues - remove them if they contain interactive elements
allPresentationElements.forEach((element) => {
const hasInteractiveChildren =
element.querySelectorAll(
'button, input, select, textarea, [tabindex], [onclick], [contenteditable]'
).length > 0
if (hasInteractiveChildren) {
element.removeAttribute('role')
}
})
// Also fix any elements with aria-rowcount="-1" which is problematic
const negativeRowCountElements = document.querySelectorAll(
'[aria-rowcount="-1"]'
)
negativeRowCountElements.forEach((element) => {
element.removeAttribute('aria-rowcount')
})
// Ensure proper table structure
const tableElements = document.querySelectorAll('table')
tableElements.forEach((table) => {
if (!table.getAttribute('role')) {
table.setAttribute('role', 'table')
}
// Ensure table headers have proper scope
const headerCells = table.querySelectorAll('th')
headerCells.forEach((th) => {
if (!th.getAttribute('scope')) {
th.setAttribute('scope', 'col')
}
})
})
// Add proper ARIA labels to interactive elements
const interactiveElements = document.querySelectorAll(
'button, input, select, textarea, [contenteditable]'
)
interactiveElements.forEach((element) => {
if (
!element.getAttribute('aria-label') &&
!element.getAttribute('aria-labelledby')
) {
const textContent = element.textContent?.trim()
if (textContent) {
element.setAttribute('aria-label', textContent)
}
}
})
}
// Run the fix immediately
fixAriaIssues()
// Run it again after a short delay to catch any dynamically created elements
setTimeout(fixAriaIssues, 100)
setTimeout(fixAriaIssues, 500)
setTimeout(fixAriaIssues, 1000)
setTimeout(fixAriaIssues, 2000)
// Set up a periodic check to ensure accessibility fixes are maintained
if (!this.ariaCheckInterval) {
this.ariaCheckInterval = setInterval(fixAriaIssues, 3000)
}
// Set up a MutationObserver to continuously monitor for new problematic elements
if (!this.ariaObserver) {
this.ariaObserver = new MutationObserver((mutations) => {
let shouldFix = false
mutations.forEach((mutation) => {
if (
mutation.type === 'attributes' &&
(mutation.attributeName === 'role' ||
mutation.attributeName === 'aria-rowcount')
) {
shouldFix = true
}
if (mutation.type === 'childList') {
mutation.addedNodes.forEach((node) => {
if (node.nodeType === Node.ELEMENT_NODE) {
const element = node as Element
if (
element.hasAttribute('role') ||
element.hasAttribute('aria-rowcount')
) {
shouldFix = true
}
}
})
}
})
if (shouldFix) {
setTimeout(fixAriaIssues, 50)
}
})
// Start observing the entire document for changes
this.ariaObserver.observe(document.body, {
childList: true,
subtree: true,
attributes: true,
attributeFilter: [
'role',
'aria-rowcount',
'aria-colcount',
'aria-multiselectable'
]
})
}
} }
async ngAfterContentInit() { async ngAfterContentInit() {