fix(a11y): name the three hidden file inputs - #253
Closed
rubenvdlinde wants to merge 3 commits into
Closed
Conversation
added 3 commits
August 9, 2026 15:47
…rated files tracked under their own ignore rule Three mechanical gate failures, each with a real defect behind it. gate-43 table-headers — the theming audit log renders a four-column `<table>` whose `<th>`s carried no `scope=`. With no scope, a screen reader cannot announce which header belongs to a cell, so the log — the artefact this app offers as WCAG-EM audit evidence — was itself not readable by the users it exists to serve. `scope="col"` on all four. gate-44 autocomplete-attr — the three email-footer fields declared no autocomplete purpose. They are INSTANCE-WIDE configuration (the organisation named in every outgoing mail, and that organisation's public statement URLs), not personal details of the admin filling the form, so `autocomplete="off"` is the accurate declaration and not a silencer: it stops a browser offering the admin's own profile data as the value for a setting that applies to everyone on the instance. Worth recording for whoever meets this gate next: it matched these three only because the substring `email` appears in their ids (`nldesign-email-footer-org-name`), and not one of them collects an email address. The attribute is right on its own merits; the match that found it was not. gate-29 gitignore-then-commit — `.gitignore:61` has ignored `/phpmetrics/` for as long as the rule has existed, while 81 generated report files sat tracked underneath it. `composer phpmetrics` regenerates the directory and CI sets `enable-phpmetrics: true`, so every run could rewrite files git was still watching. Nothing links to the committed HTML; the rule was right and the tracking was the oversight. Untracked, not re-ignored. Measured with hydra-gates 48c88ba1e0d049f8f38538c33e790d3e603c55d0: gate-43 FAIL 1 -> PASS, gate-44 FAIL 3 -> PASS. Both proven able to fail on this tree first — a planted unscoped `<th>` and a planted unlabelled input both fired in a control run before these fixes.
…nd the spec that mandated the wrong one
gate-10 initial-state: FAIL 4 -> PASS.
js/admin.js pulled four server-rendered values off the DOM with
`getAttribute('data-…')`: `tokenSets`, `currentTokenSet`,
`activePreview` and `iconPackSource`. ADR-004 rules that out —
`IInitialState::provideInitialState()` in PHP, `loadState()` in JS —
and the reason is not style. A data attribute breaks on CSP-hardened
instances, and it breaks SILENTLY: any markup change that moves or
renames the carrying element leaves admin.js parsing an empty string
and rendering as though the server had sent nothing at all. The app
already did this correctly one file over, in js/preview-banner.js.
THE SPEC MANDATED THE DEFECT
----------------------------
`openspec/specs/admin-settings/spec.md` carried a requirement titled
"Data Attributes for JavaScript Initialization" whose three scenarios
required exactly what gate-10 forbids, down to
`p(json_encode(...))` on `data-token-sets`. Fixing only the code would
have left the canonical spec instructing the next person to put it
back. The requirement is rewritten to mandate initial state, with the
contradiction and its resolution recorded in place rather than
silently overwritten.
WHILE IN THE FILE
-----------------
`notify()` fell back to `notify(message)` instead of
`OC.Notification.showTemporary(message)` — an unconditional infinite
recursion on any instance where `OCP.Toast` is absent and the NC<=33
API is present. The RangeError was swallowed by the surrounding
try/catch, so every toast on that path silently became a console line.
EVIDENCE
--------
- hydra-gates 48c88ba1e0d049f8f38538c33e790d3e603c55d0: gate-10
FAIL 4 -> PASS; gate-19 unchanged at 156, so rewriting the spec
added no e2e debt (three scenarios renamed, one added, all covered
or reason-excluded).
- vitest 81/81, having been 81/81 before the change and 75/81 midway:
six tests in admin-a11y and admin-dark-mode failed the moment
admin.js stopped reading attributes, which is the positive control
that the new read path is the one under test. All five DOM fixtures
now provide initial state the way the settings page does.
- The two e2e tests that asserted the ATTRIBUTE now assert the
initial-state element AND that the attribute is gone — on the item,
not on the panel rendering, because the panel renders correctly
with both mechanisms present and that is the state being retired.
- PHP 8.4: phpcs clean, phpmd clean, phpstan "No errors", psalm "No
errors found". (The host runs 8.2 and this repo's floor is 8.3, so
the analysers cannot run on the host at all — they were run in a
php:8.4-cli container against this worktree's own vendor/.)
gate-40 form-label-association: FAIL 3 -> PASS. The three `<input type="file">` controls behind the Upload / Choose font / Upload configuration buttons carried no accessible name. Each is `style="display:none"` and is only ever opened by `.click()` from its adjacent visible button. WHY THIS IS NOT AN INERT ATTRIBUTE ---------------------------------- ConductionNL/.github#273 documents a gate-40 finding that genuinely cannot be closed: an input carrying `aria-hidden="true"`, where a name can never be announced because the element is not in the accessibility tree at all, and where the only remediations regress gate-37. That issue explicitly separates THIS repo's three findings from that shape: "nldesign's three gate-40 findings are the same shape but use style="display:none", not aria-hidden. Excluding display:none would be genuinely unsafe — a style can be toggled by JS, so the element may well be exposed at runtime." That is the whole argument for naming them. `display:none` removes the control from the tree WHILE it is hidden, but one style change from script or a user stylesheet puts it back, and at that moment an unnamed file input is a real WCAG 4.1.2 failure. The name costs nothing while hidden and is correct the instant it is not. Each label says what the input accepts rather than repeating the button ("Token set file to upload (NL Design CSS or W3C Design Tokens JSON)"), and all three go through `$l->t()`. No visible label is overridden: these inputs have no visible label and no default slot to name themselves from, which is the case where an aria-label would have been a regression rather than a fix. Measured with hydra-gates 48c88ba1e0d049f8f38538c33e790d3e603c55d0: gate-40 FAIL 3 -> PASS, with gates 31/35/39/41/43/44 unchanged at PASS so the added attributes did not silence a neighbouring rule. vitest 81/81.
Contributor
Quality Report — ConductionNL/nldesign @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 2/2 | |||
| PHPUnit | ❌ | ||||
| Newman | ✅ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-09 14:33 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
gate-40 form-label-association: FAIL 3 → PASS. Package
48c88ba1e0d049f8f38538c33e790d3e603c55d0.The three
<input type="file">controls behind the Upload / Choose font / Upload configuration buttons carried no accessible name. Each isstyle="display:none"and is only ever opened by.click()from its adjacent visible button.Why this is not an inert attribute
I nearly left this one red. ConductionNL/.github#273 documents a gate-40 finding that genuinely cannot be closed: an input carrying
aria-hidden="true", where a name can never be announced because the element is not in the accessibility tree at all, and where both remediations regress gate-37.That issue explicitly separates this repo's three findings from that shape:
That is the whole argument for naming them.
display:noneremoves the control from the tree while it is hidden, but one style change from script or a user stylesheet puts it back, and at that moment an unnamed file input is a real WCAG 4.1.2 failure. The name costs nothing while hidden and is correct the instant it is not — which is exactly what distinguishes it from thearia-hiddencase, where the name can never do anything at all.Each label says what the input accepts rather than repeating the button, and all three go through
$l->t().No visible label is overridden. These inputs have no visible label and no default slot to name themselves from — the case where an
aria-labelwould have been a regression rather than a fix.Evidence
<input type="text">took this gate from 3 to 4 findings on this tree in a control run.After this, the only gates still red on a full-scope run of this repo are gate-7 (2) and gate-19 (156).
No waivers, no baselines, no threshold changes.