Skip to content

gates 12/32/38: two text-extraction false positives and one that scores a comment #236

Description

@rubenvdlinde

Three text-extraction defects, measured on ConductionNL/softwarecatalog against hydra-gates@main (756fe89), run over the full tree. Two produce false positives on correct code; the third makes a gate score a comment.


1. gate-12 (nc-input-labels): [^>]* stops at the arrow in an arrow function

run-hydra-gates.sh:

grep -oE '<NcSelect[^>]*>' 2>/dev/null \
    | while IFS= read -r tag; do
        if ! echo "${tag}" | grep -qE "(input-label|inputLabel|aria-label-combobox|ariaLabelCombobox)"; then

[^>]* stops at the first >. In an NcSelect with a reducer — which is the idiomatic way to keep the stored value a plain string rather than the {label,value} option object — that first > is the arrow:

<NcSelect
  v-model="emailSettings.transportType"
  :reduce="option => option.value"      <-- extraction ends HERE
  :options="[…]"
  input-label="Transport Type"          <-- never seen
  :disabled="!emailSettings.enabled" />

The extracted "tag" ends before input-label can appear, so a correctly-labelled select is reported as unlabelled. The finding text in the log shows the truncation directly — it literally ends at :reduce="option =>.

Measured: both gate-12 findings on softwarecatalog are this. Both selects carry input-label. The gate has been red on this repo for a WCAG rule the repo already satisfies.

This is the same shape as the gate-9 admin rule's [^)]* in #198, and it fails the same two ways at once: any prop value containing > truncates the tag (false positive), and the truncated remainder is discarded rather than re-scanned.

Suggested fix: extract to the tag's real close — track quoting, or at minimum <NcSelect(\s+[^>]*?)?/?> with quoted-value awareness. Whatever the mechanism, the unit tests should include a tag with an arrow function before the label prop, since that is the ordering the false positive needs.


2. gate-32 (semantic-controls): matches markup inside Vue comments

The fix for gate-32 on softwarecatalog added an explanatory comment above each repaired element:

<!-- role/tabindex/keydown rather than a bare <div @click>: picking the
     merge target is the consequential choice in this dialog … -->
<div v-for="obj in availableObjects"
     role="option"
     tabindex="0"
     @click="selectTargetObject(obj)"
     @keydown.enter.prevent="selectTargetObject(obj)">

The gate still reported all three files — and the reported tag was <div @click> with no attributes, i.e. the comment, not the element. Rewording the comments to prose ("rather than a click-only div") cleared the gate with no change to the markup.

This was confirmed, not assumed: the same tree was measured before and after the comment rewording, with the element markup byte-identical across both runs. gate-32 went FAIL(3) → PASS.

Consequence in both directions:

  • a correct element whose comment describes what it replaced is reported (false positive), and
  • a genuinely bad element can be "explained away" by a comment — which is worse, because the natural next step for someone chasing this gate is to write a comment.

Same class as #184: a checker that greps a string literal misses every constant and matches every comment.

Suggested fix: strip <!-- … --> from the template before scanning, as the other Vue-template gates do.


3. gate-38 (skip-link): a Nextcloud admin-settings section is not a root component

gate-38 reports templates/settings/admin.php on softwarecatalog:

Util::addScript($appId, $appId . '-settings');
Util::addStyle($appId, 'main');
?>
<div id="softwarecatalog-settings"></div>

That template is registered via lib/Settings/SoftwareCatalogAdmin.php (an OCP\Settings\ISettings) and is rendered inside core's settings frame, which already supplies the skip link and the <NcContent> shell. It is not a root component and never controls the page's landmark structure. Adding a second skip target inside the settings shell would be an accessibility regression, so the gate's advice is actively wrong here.

Suggested fix: exclude templates that are only reachable through an ISettings::getForm() implementation — i.e. any templates/settings/*.php whose name appears in a new TemplateResponse(..., 'settings/…') returned from a class implementing OCP\Settings\ISettings. A cheaper approximation: skip templates/settings/.


Why these were reported rather than worked around

None was silenced in the app. No waiver, no exclude tag, no baseline entry. softwarecatalog's PR #458 leaves gate-12 and gate-38 red and says why, with the call-path evidence above.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions