Rule: allow input nested in label (#1170)#1852
Rule: allow input nested in label (#1170)#1852ShivaniNR wants to merge 6 commits intohtmlhint:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the input-requires-label rule to support nested inputs within label tags, effectively treating them as valid without requiring an explicit for attribute. The implementation uses a depth-tracking mechanism for labels during parsing. Comprehensive tests have been added to verify nesting behavior and edge cases. Feedback indicates that the documentation update incorrectly categorizes the new valid example as a rule violation and suggests fixing swapped section headers.
| ```html | ||
| <label for="some-id"/><input id="some-id" type="password" /> | ||
| <input id="some-id" type="password" /> <label for="some-id"/> | ||
| <label><input type="password" /></label> |
There was a problem hiding this comment.
This example is currently placed under the "considered rule violations" section (starting at line 31). Since the goal of this pull request is to allow nested inputs as a valid pattern, this example should be moved to the "not considered rule violations" section (starting at line 22).
Additionally, it appears that the section headers in this documentation file are currently swapped (line 22 and line 31), which likely caused the confusion. Correcting the headers would improve the overall clarity of the documentation.
There was a problem hiding this comment.
Fixed. The section headers were indeed swapped, "not violations" had violations, and vice versa. Swapped them back, and the nested example is under the "not considered rule violations" section.
- treat input nested in label as implicitly labeled - add tests for nested, multi-input, and edge cases - document the nested form Per HTML spec, a label's labeled control is either what for= points to, or its first labelable descendant. The rule previously only honored for/id matching. Fixes htmlhint#1170
548b47a to
1b501b2
Compare
There was a problem hiding this comment.
Pull request overview
Updates the input-requires-label rule so that <input> elements nested within an open <label> scope are treated as labeled (avoiding warnings), addressing #1170’s request to support implicit label associations.
Changes:
- Track
<label>nesting depth viatagstart/tagendand skip warnings for inputs encountered while nested. - Add new unit tests for nested inputs and several malformed/self-closing label scenarios.
- Update the rule documentation examples to include a nested
<label><input/></label>pattern.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| website/src/content/docs/rules/input-requires-label.mdx | Adds a nested-label example (currently placed in the violations section). |
| test/rules/input-requires-label.spec.js | Adds coverage for nested inputs and label depth edge cases. |
| src/core/rules/input-requires-label.ts | Implements label nesting depth tracking and uses it to suppress warnings for nested inputs. |
| dist/core/rules/input-requires-label.js | Compiled output reflecting the rule changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| inputTags.forEach((inputTag) => { | ||
| if (!hasMatchingLabelTag(inputTag)) { | ||
| if (!inputTag.nested && !hasMatchingLabelTag(inputTag)) { | ||
| reporter.warn( |
There was a problem hiding this comment.
The new nested short-circuit (if (!inputTag.nested && !hasMatchingLabelTag(...))) means any input seen while labelDepth > 0 will skip validation entirely. Once depth tracking is limited to implicit labels (no for), consider whether inputs inside explicit labels should still be validated against labelTags (they typically still need their own matching label or other accessible name).
There was a problem hiding this comment.
Resolved by the fix. Since labelDepth no longer increments for labels with for, inputs inside explicit labels won't be marked as nested and will go through normal hasMatchingLabelTag validation.
| ```html | ||
| <label for="some-id"/><input id="some-id" type="password" /> | ||
| <input id="some-id" type="password" /> <label for="some-id"/> | ||
| <label><input type="password" /></label> | ||
| ``` |
There was a problem hiding this comment.
The nested-input example was added under “considered rule violations”, but this PR changes the rule to allow an <input> nested in a <label> without needing for/id. Move this example to the non-violation section (and ensure the violation/non-violation examples match the rule’s actual behavior).
There was a problem hiding this comment.
Fixed along with the header swap mentioned above.
| if (tagName === 'label') { | ||
| // a self-closing <label/> opens no scope and emits no tagend | ||
| if (!event.close) { | ||
| labelDepth++ | ||
| } | ||
| if ('for' in mapAttrs && mapAttrs['for'] !== '') { | ||
| labelTags.push({ event: event, col: col, forValue: mapAttrs['for'] }) | ||
| } |
There was a problem hiding this comment.
labelDepth is incremented for every non-self-closing <label>, including labels with a for attribute. Per the HTML spec, implicit labeling via nesting only applies when for is not specified; if for exists, the label targets the referenced control and a nested <input> is not labeled by that <label>. This will incorrectly suppress warnings for <label for="other-id"><input ...></label> (and also for for=""). Track depth only for labels that can provide implicit labeling (e.g., for not present), or store a stack of label scopes with a flag indicating whether they are implicit.
There was a problem hiding this comment.
Fixed. labelDepth now only increments for labels without a for attribute. Labels with for go to labelTags for explicit matching only, so correctly warns.
|
I think it should be: <label>
Password
<input type="password" />
</label>`we really need to include this check in the rule logic too though otherwise users wouldn't get alerted to non-accessible usage... |
- Labels with a for attribute target the referenced control, not nested
inputs. Only increment labelDepth for labels without for.
- Move <label for="other-id"><input> test to error cases.
- Swap doc section headers (were backwards) and add visible text to
nested label example.
Addresses review feedback on htmlhint#1852.
…shivaninr/input-requires-label-nested
…ithub.com/ShivaniNR/HTMLHint into dev/shivaninr/input-requires-label-nested
Good catch. I have updated the doc example to include visible text ( Regarding the rule also checking for empty label text, that would require tracking text nodes between label open/close during parsing, which is a bigger scope change. Would it make sense to open a separate issue for that? Happy to work on it as a follow-up. |
|
Thanks for your work on this. Nice job!
Yes please, that'd be great. |
Short description of what this resolves
The
input-requires-labelrule only checks for<label for="...">/<input id="...">associations. Per the HTML spec, an<input>nested directly inside a<label>is implicitly labeled, but the rule warns on this valid pattern.Fixes #1170
Proposed changes
tagstart/tagendevents. An input inside an open<label>is treated as labeled and skips the warning.<label/>doesn't open a scope (parser emits notagendfor it), and stray</label>tags can't underflow the depth counter.