Skip to content

Fixes #3045 - #3054

Merged
erikdarlingdata merged 2 commits into
devfrom
fix/3045-ag-rollup-tile-aria
Sep 6, 2026
Merged

erikdarlingdata merged 2 commits into
devfrom
fix/3045-ag-rollup-tile-aria

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

Fixes #3045.

rollup() in wwwroot/js/pages/ag.js built each of its three tiles as a number div and a label div with nothing tying them together. The text was in the accessibility tree, in the correct reading order, and the meaning was carried by text rather than colour — nothing was unavailable. What was missing is the programmatic relationship, the same thing #3038 fixed on the fleet page and deliberately left here because the two pages share no code path.

The label now carries a stable label-derived id (rollup-groups-lbl, rollup-reporting-servers-lbl, rollup-views-lbl) and the number points at it with aria-describedby. Wired once in the tile closure, at no call site, so a tile cannot be added half-wired. Nothing is added to, removed from or reordered in the rendered text; no stylesheet in this wwwroot keys on an id or on an aria-* attribute (the only id selectors are the shell's six: app, sidebar, main, statusbar, server-list, view-list), so nothing moves visually and no getElementById lookup is shadowed.

The decision this carried: hoist, not duplicate

#3031's closing note said "the util.js hoist is the shape", and #3045 corrects it — there was no hoist. rollupTextId and the wired closure both lived in fleet.js, and util.js exposed no tile or id helper, so creating the shared helper was the decision.

rollupTextId moves to util.js verbatim (byte-identical function body; the removal from fleet.js and the insertion into util.js matched exactly), and both pages import it. The reason is the de-duplication rule it carries, which is the subtle part: two tiles sharing a label emit a duplicate id, and aria-describedby resolves a duplicate to the first match — a silent mis-association rather than a visible break. AG's three labels are all distinct today, so the used set is inert on this page — which is exactly why a local copy would have been written without it, and would have stayed correct right up until someone added a fourth tile that repeated a label. Eight lines with no page-specific knowledge is a poor trade for a second copy of a rule that fails quietly.

The alternative — duplicating the derivation in ag.js — is cheaper to review in isolation and leaves the pages free to diverge. Nothing here wants to diverge: the two rollups differ in what they qualify, not in how an id is spelled.

What did not come across from the model

The sub-line machinery. fleet.js's tile takes sub and title because that rollup carries a coverage sub-line on one tile and a long coverage note; six of its seven tiles have no sub-line either. The AG rollup has neither, so a second id would point at an element that does not exist. subId is absent, and its absence is pinned rather than left to the next reader's restraint.

Why the label rides in aria-describedby

.num is a plain div. ARIA prohibits naming role=generic, so aria-label / aria-labelledby set there is invalid and may be dropped; a description is a supported property on it. Description is the carrier that works without inventing a widget role for a static figure. Page-independent, taken from #3038 rather than re-derived.

The guard fails on an unwired tile, not merely passes on a wired one

Darling/Darling.Tests/RollupTileDescriptionTests.cs, six tests. A positive-only check is why this page survived #3031 — asserting that the fleet tiles carry a description would have stayed green through the entire life of this defect. So the population is derived from the shipped source: the modules under test are found by scanning wwwroot/js for rollup figures, every figure found must carry the relationship, and a page that grows a rollup tomorrow is swept in with no edit to the test. Every enumeration has a floor, because an empty match is the one way a check like this reads as clean while proving nothing.

The figure regex is scoped to el("div", …), so compose.js's numeric td.num grid cell — a table cell whose meaning comes from its column header, not a sibling label — is not swept in.

Nine mutations, each proven to have landed (the driver asserts its anchor is present, and refuses to run if not), each failing a different named assertion:

mutation reds
aria-describedby removed from the AG figure EveryRollupFigure_CarriesADescription, TheAgNumbers_DescribeThemselvesWithTheirLabelsId
a fourth tile hand-rolled outside the closure, carrying a description EveryRollupPage_BuildsEveryTileFromOneClosure only — the sweep passes it, which is why the one-closure pin exists
the same fourth tile with no description both of the above
the de-duplication loop removed from the shared helper TheSharedDerivation_DeDuplicatesWithinARender
the render's used-id set dropped at the AG call site EveryIdDerivationCall_IsHandedTheRendersUsedIdSet
the derivation duplicated into ag.js (the rejected option) TheIdDerivation_ExistsExactlyOnce_AndEveryFigurePageImportsIt
fleet.js's sub-line machinery copied over by resemblance TheAgNumbers_DescribeThemselvesWithTheirLabelsId
ag.js restored to what dev ships — the defect exactly as reported four tests, the sweep among them
.num renamed on both pages, so the enumeration matches nothing the floors: expected at least 2 rollup figures under wwwroot/js, found 0

Mutations ran against a copy of the tree in a sandbox, after this commit, so a restore could not eat uncommitted work. The test source itself was never mutated.

Darling.Tests cannot execute on macOS, so the six tests were run by compiling this exact file into a throwaway net10.0 xunit v3 host staged outside the tree: Total: 6, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0. The real Darling/Darling.Tests/Darling.Tests.csproj builds clean with the file in place (-p:EnableWindowsTargeting=true, 0 errors, 0 warnings attributable to it), re-checked after merging origin/dev.

Behaviour, under a DOM shim

The source pins cannot see what the module builds, so the shipped ag.js and util.js were run under a minimal DOM shim in node. The only change to ag.js was export on the module-private function rollup(d); util.js is byte-identical, and panels.js is a stub (rollup() never touches VIZ).

Each of the three figures is described by its own label, every reference resolves to an element in the built tree, no duplicate ids, no name set on any generic div, no title anywhere on the tile, and the ids are identical across a re-render (the page re-renders on a 60s cadence).

The duplicate-label hazard was demonstrated rather than asserted, with the control #3038 ran:

variant result
shipped Groups→rollup-groups-lbl, Reporting servers→rollup-reporting-servers-lbl, Views→rollup-views-lbl
two tiles sharing a label, guard in place (control) the second gets rollup-groups-lbl-2 and resolves to its own label — green
two tiles sharing a label, guard removed duplicate rollup-groups-lbl; the third figure's description resolves to the first match, so the number reading 9 is described by the label belonging to the tile reading 4 — red

Not verified

  • Nothing was rendered. No browser loaded the page, no live Darling service was involved, and /api/ag was never called — rollup() was handed a payload object directly.
  • No screen reader was run. Nothing here proves what NVDA, JAWS or VoiceOver say. .num is a non-focusable generic element and screen readers vary in whether they announce a description on one at all. This establishes the programmatic relationship 1.3.1 asks for; it does not promise a changed announcement.
  • IDREF resolution was my own, not an engine's. Fixes #3031 #3038 read Chrome's ariaDescribedByElements; this run indexes the built tree first-match-wins in the shim. The resolution rule is modelled, not observed.
  • No engine at all, therefore no engine differences, and no light-mode or high-contrast check. The claim that nothing moves visually is the stylesheet grep above, not an observation.
  • The AG payload field names (distinct_ag_count, reporting_server_count, availability_group_count) are unchanged by this PR and were not re-verified against DarlingAgReader.

CHANGELOG entry, for whoever owns the [Unreleased] block

CHANGELOG.md is untouched here — every lane appends to the same block. Under ### Fixed:

- **The AG page's rollup numbers now say what they count, and the id derivation is shared rather than copied** ([#3045]) - `rollup()` in `wwwroot/js/pages/ag.js` built each tile's number and its label as sibling divs with nothing tying them together: the association was inferable from reading order but not determinable, the defect [#3038] fixed on the fleet page and deliberately left here because the two pages share no code path. The label now carries a stable label-derived id and the number describes itself with it, wired once in the tile closure so no call site can opt a tile out. **`rollupTextId` moved from `fleet.js` to `util.js` rather than being copied into it.** The de-duplication rule it carries is the subtle part - two tiles sharing a label emit a duplicate id, and `aria-describedby` resolves a duplicate to the FIRST match, a silent mis-association rather than a visible break - and AG's three labels are all distinct today, so the `used` set is inert on that page and a local copy would have been written without it, staying correct until someone added a fourth tile that repeated a label. The label rides in `aria-describedby` rather than `aria-label`/`aria-labelledby` because `.num` is a plain div: ARIA prohibits NAMING `role=generic`, so a name set there is invalid and may be dropped, while a description is a supported property on it. `fleet.js`'s sub-line machinery did NOT come across - that rollup qualifies one number with coverage and hovers a long note; these three counts do neither, so a second id would point at nothing. `RollupTileDescriptionTests` derives its population from the shipped modules, so every rollup figure under `wwwroot/js` must carry a description and a page that grows a rollup is covered without the test being edited, with a floor under each enumeration so an empty match cannot read as clean - **a positive-only check is exactly what let this page survive [#3031]**. Verified by nine mutations each failing a different named assertion, including `ag.js` restored to the defect as reported, and by running the shipped module under a DOM shim; no screen reader was run and no browser rendered the page.

Needs a link definition: [#3045]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/3045

…3045)

The AG rollup's three tiles built the number and its label as sibling divs
with nothing tying them together, the defect #3038 fixed on the fleet page.
The label now carries a stable id and the number describes itself with it.

rollupTextId moves from fleet.js to util.js rather than being copied: the
de-duplication rule it carries is subtle (a duplicate id resolves to the
first match, mis-associating silently), AG's labels are all distinct today,
and a local copy would be written without the guard and stay correct until a
fourth tile repeated a label.

The guard derives its population from the shipped modules — every rollup
figure under wwwroot/js must carry a description, with a floor under the
enumeration — because a positive-only check is what let this page survive
#3031.
@claude

claude Bot commented Sep 6, 2026

Copy link
Copy Markdown

Reviewed. No correctness, security, or parity issues found.

  • Correctness: ag.js's tile closure correctly wires lblId/aria-describedby on the number and id on the label; braces/returns are balanced, and the usedIds Set is scoped per rollup(d) call so it can't leak state across the page's 60s re-render. rollupTextId moved to util.js is byte-identical to the version removed from fleet.js (only export added). Verified against the actual source that NumberFigure/TileContainer regexes in the new test file match exactly the two div-based rollup tiles (fleet.js, ag.js) and correctly skip compose.js's td.num grid cell (different tag) and panels.js's dynamic class expressions (no literal "num" token) — so the derived-population tests won't silently pass over an unswept page or false-positive on unrelated "num"/"tile" usage elsewhere in wwwroot/js.
  • Lite/Darling parity: N/A here — this is Darling's web dashboard (HTML/ARIA); Lite is a WPF/Avalonia desktop app with no equivalent rollup-tile UI or AutomationProperties usage, so there's no counterpart to update.
  • Security: labels passed into rollupTextId ("Groups", "Reporting servers", "Views", and fleet's fixed labels) are static strings, not server-sourced data, so no injection surface; ids/attributes are set via el()'s setAttribute/textContent path (never innerHTML), consistent with this file's existing R4 rule.
  • Performance: negligible — one Set and a handful of string ops per render, same shape as the existing fleet.js pattern this generalizes.

The new RollupTileDescriptionTests.cs derives its module population from the shipped wwwroot/js tree rather than a hardcoded list (with floors under each enumeration), which is exactly the right shape to avoid repeating the "positive-only check that survives the defect" failure mode called out in the PR description for #3031.

No blocking issues; nothing else to flag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant