Skip to content

fix(gate-37,gate-43): tabindex="-1" is NOT focusable, and one scope= does not green a table (#222) - #251

Merged
rubenvdlinde merged 4 commits into
mainfrom
fix/gate-37-43-focusability-and-per-header-scope
Aug 8, 2026
Merged

fix(gate-37,gate-43): tabindex="-1" is NOT focusable, and one scope= does not green a table (#222)#251
rubenvdlinde merged 4 commits into
mainfrom
fix/gate-37-43-focusability-and-per-header-scope

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Closes #222.

Two gates whose remediation advice regressed the thing they measure.

gate-37 — tabindex="-1" counted as focusable

The check was grep -qE '(:?tabindex|v-bind:tabindex)='any value. But tabindex="-1" is the attribute that removes an element from the tab order: the one value that proves the opposite of what the gate concluded. The gate's own subject ("hidden from AT and still keyboard-reachable") was inverted for every element that had already been fixed.

The canonical hidden-file-input pattern trips it exactly:

<input type="file" :aria-hidden="true" tabindex="-1" @change="">

The advice was to remove aria-hidden (exposing an unnamed control to screen readers) or to remove tabindex="-1" (putting a control screen readers cannot see back in the tab order — the very defect this gate exists to catch). Both remediations regress accessibility.

Measured: nextcloud-vue 2 → 0. Both were tabindex="-1" (CnFilesWidget's file input, CnCalendarEventCreate's submit bridge). Both 100% false.

Still flagged: tabindex="0"/positive, native focusables with no negative tabindex, interactive role=, and a bound :tabindex="expr" whose value cannot be read — unknown is not "safe", so the old behaviour is kept there.

gate-43 — one scope= greened the whole table

if re.search(r'<th\b[^>]*\bscope\s*=', body): continue   # whole table OK

A single scope= anywhere accepted every other header. Negative control: removing exactly one scope= from a passing table still reported PASS. The rule is now any unscoped named header fails the table. That experiment is now an assertion — test_removing_exactly_one_scope_flips_the_verdict.

A header with no name is not a header

Tightening to per-header immediately produced 8 findings in openconnector, a repo the old rule passed — and all 8 were the same false positive: the empty spacer column carrying a drag handle or row-actions menu (<th aria-hidden="true" />, <th />).

scope= declares which cells a header names. A header with no name names nothing, so the attribute is inert and demanding it is remediation theatre — the same trap as gate-40's aria-label advice. Exempt: aria-hidden="true", role="presentation"/none, empty content. A header with text and no scope= still fails. openconnector: back to 0.

Counting

One finding per table, not per <th>, with unscoped=N/M on the line. The number stays a count of defect sites and comparable with what this gate reported before. A finding count is not a defect count, and the two must not be silently swapped mid-repair.

Fleet effect, measured over 10 repos

gate-43 numbers identical before and after (opencatalogi 15, docudesk 11, procest 28, openbuild 8, openregister 56, pipelinq 43, doriath 14, openconnector 0). The tightening and the exemption are not cancelling each other — the tests prove each arm independently; the fleet simply has no partially-scoped table whose unscoped header is named.

Mutation-checked

reverted fix suite result
gate-37 "any tabindex is focusable" 6 failures
gate-43 "at-least-one scope" 5 failures
gate-43 unnamed-header exemption widened to all 7 failures

Also caught by mutation rather than by reading: an optional (?:(.*?)</th>)? body group let a self-closed <th /> reach past itself to the first </th> in the file and swallow the next header whole — two headers counted as one.

Also

  • Both implementations moved out of the runner into tested helpers (check_aria_hidden_focusable.py +24 tests, check_table_headers.py +23 tests).
  • Both are quote-aware: the old [^>]* attribute runs ended a tag at the > inside :title="a > b", hiding attributes that followed.
  • Both strip comments and <script>/<style> — markup that does not ship is not a control.
  • A missing helper now reports SKIP-wiring, not PASS, for both gates (fix(gates): 16 gates reported PASS when their helper never ran — 2 are authorization gates #147).

Suites: 29 helper suites pass, 59 entry-point tests pass.

rubenvdlinde and others added 4 commits August 8, 2026 15:36
…does not green a table (#222)

Two gates whose remediation advice regressed the thing they measure.

GATE-37 — `tabindex="-1"` counted as focusable
---------------------------------------------
The check was `grep -qE '(:?tabindex|v-bind:tabindex)='` — ANY value. But
`tabindex="-1"` is the attribute that REMOVES an element from the tab order:
the one value proving the opposite of what the gate concluded. The gate's own
subject ("hidden from AT and STILL keyboard-reachable") was inverted for every
element that had already been fixed.

The canonical hidden-file-input pattern trips it exactly:

    <input type="file" :aria-hidden="true" tabindex="-1" @change="…">

The advice was to remove `aria-hidden` (exposing an unnamed control to screen
readers) or to remove `tabindex="-1"` (putting a control screen readers cannot
see BACK in the tab order — the very defect this gate exists to catch). Both
remediations regress accessibility.

Measured: nextcloud-vue 2 findings -> 0. Both were `tabindex="-1"` (CnFilesWidget's
file input, CnCalendarEventCreate's submit bridge), both 100% false.

Still flagged: `tabindex="0"`/positive, native focusables with no negative
tabindex, interactive `role=`, and a BOUND `:tabindex="expr"` whose value
cannot be read — unknown is not "safe", so the old behaviour is kept there.

GATE-43 — one `scope=` greened the whole table
----------------------------------------------
    if re.search(r'<th\b[^>]*\bscope\s*=', body): continue   # whole table OK

A SINGLE `scope=` anywhere accepted every other header in the table. Negative
control: removing exactly one `scope=` from a passing table still reported
PASS. The rule is now "any unscoped NAMED header fails the table". That
experiment is now an assertion — test_removing_exactly_one_scope_flips_the_verdict.

A HEADER WITH NO NAME IS NOT A HEADER. Tightening to per-header immediately
produced 8 findings in openconnector, a repo the old rule passed, and ALL
EIGHT were the same false positive: the empty spacer column carrying a drag
handle or a row-actions menu (`<th aria-hidden="true" />`, `<th />`). `scope=`
declares which cells a header NAMES; a header with no name names nothing, so
the attribute is inert and demanding it is remediation theatre — the same trap
as gate-40's `aria-label` advice. Exempt: `aria-hidden="true"`,
`role="presentation"`/`none`, and empty content. A header WITH text and no
`scope=` still fails. openconnector: back to 0.

COUNTING: one finding per TABLE, not per `<th>`, with `unscoped=N/M` on the
line. The number stays a count of defect sites and comparable with what this
gate reported before. A finding count is not a defect count, and the two must
not be silently swapped mid-repair.

Fleet effect of both changes together, measured over 10 repos: gate-43 numbers
IDENTICAL before and after (opencatalogi 15, docudesk 11, procest 28,
openbuild 8, openregister 56, pipelinq 43, doriath 14, openconnector 0). The
tightening and the exemption are not cancelling each other — the tests prove
each arm independently; the fleet simply has no partially-scoped table whose
unscoped header is named.

Both implementations moved out of the runner into tested helpers:
  scripts/lib/check_aria_hidden_focusable.py  + 24 tests
  scripts/lib/check_table_headers.py          + 23 tests

Both are quote-aware now: the old `[^>]*` attribute runs ended a tag at the
`>` inside `:title="a > b"`, hiding attributes that followed. Both strip
comments and script/style blocks — markup that does not ship is not a control.

MUTATION-CHECKED. Every fix reverted in turn, and the suites go red:
  * gate-37 "any tabindex is focusable"        -> 6 failures
  * gate-43 "at-least-one scope"               -> 3 failures (5 with exemption)
  * gate-43 unnamed-header exemption widened   -> 7 failures
Also caught by mutation, not reading: an optional `(?:(.*?)</th>)?` body group
let a self-closed `<th />` reach past itself to the FIRST `</th>` in the file
and swallow the next header whole — two headers counted as one.

A missing helper now reports SKIP-wiring, not PASS, for both gates (#147).

Suites: 29 helper suites pass, 59 entry-point tests pass.

Refs #222
… not take the run down

Adopts the gate-19 / #249 signalling convention for both new helpers.

Both call sites started as `>> log 2>/dev/null || true`, which discards the
traceback AND the failure. A crashed helper leaves an empty findings log, and
an empty findings log is how these gates spell PASS — the #147 defect exactly.
Exit code is now a STATUS, findings are STDOUT, stderr is KEPT in
<log>.err, and a non-zero exit reports SKIPPED (wiring).

Also wrapped in `set +e` with the caller's flag restored. gate-19's block
turns errexit ON and leaves it on for every gate after it, though this
script's header sets only `set -u`; with errexit live a failing helper never
reaches its own `_skip` — it kills the whole runner mid-sweep. Measured on
gate-38: 21 later gates silently unreported, the run ending on the abort
guard, and the PASS lines above it reading exactly like a clean run.

New suite scripts/lib/test_gate_a11y_helper_wiring.sh — 10 assertions:
  * POSITIVE CONTROL first: with both helpers intact, a fixture app built to
    fail both gates does fail both. Everything else is only meaningful
    because these fire.
  * helper MISSING   -> SKIPPED, for each gate
  * helper CRASHING  -> SKIPPED, for each gate
  * and, separately each time, that the run still reached its COVERAGE
    summary — "did not abort" cannot be folded into "said SKIPPED", because
    an aborted run's PASS lines are indistinguishable from a clean run's.
…the errexit invariant

Resolved in favour of BOTH sides, not either:

  * KEPT from main (#225 / #261): gates 37 and 43 enumerate
    `_a11y_markup_files`, not `find src -name '*.vue'`, and guard on
    `_a11y_has_markup_dir`, not `[ -d src ]`. Taking my side wholesale would
    have silently REVERTED the glob fix for these two gates — the gates would
    have gone back to seeing nothing in a PHP-template app while reporting
    PASS. Both new helpers read markup rather than Vue specifically, so an
    `aria-hidden` focusable or an unscoped `<th>` is now caught in a .php
    template too.

  * KEPT from this branch: the tabindex="-1" and per-header-scope fixes, the
    tested helpers, and the SKIPPED-on-crash wiring.

  * DROPPED from this branch (#243): the `case $- in *e*) … set -e` restore
    dance. main established that errexit is OFF for the whole script and
    nothing may turn it on — a trailing `set -e` is an unconditional ENABLE,
    not a restore, which is the defect that left 45 gates running under an
    errexit they did not expect. Restore sites now say `set +e` only, and
    scripts/lib/test_gate_errexit_discipline.sh enforces it. My earlier
    workaround is obsolete; the underlying bug is fixed upstream.

Verified after the merge: 38 helper suites pass, errexit discipline passes
(64 verdicts for 64 declared gates, no abort banner under a crashing checker).
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.

gate-37 counts tabindex="-1" as focusable (remediation regresses a11y); gate-43 counts tables not header cells (one attribute per table = green)

1 participant