Skip to content

Rule: allow input nested in label (#1170)#1852

Open
ShivaniNR wants to merge 6 commits intohtmlhint:mainfrom
ShivaniNR:dev/shivaninr/input-requires-label-nested
Open

Rule: allow input nested in label (#1170)#1852
ShivaniNR wants to merge 6 commits intohtmlhint:mainfrom
ShivaniNR:dev/shivaninr/input-requires-label-nested

Conversation

@ShivaniNR
Copy link
Copy Markdown

Short description of what this resolves

The input-requires-label rule 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

  • Track label nesting depth via tagstart/tagend events. An input inside an open <label> is treated as labeled and skips the warning.
  • Guard against parser quirks: self-closing <label/> doesn't open a scope (parser emits no tagend for it), and stray </label> tags can't underflow the depth counter.
  • Add 7 test cases covering nested input, multi-input in one label, self-closing label, and unbalanced tags.
  • Add nested example to rule docs.

@ShivaniNR ShivaniNR requested a review from coliff as a code owner April 16, 2026 01:59
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
@ShivaniNR ShivaniNR force-pushed the dev/shivaninr/input-requires-label-nested branch from 548b47a to 1b501b2 Compare April 16, 2026 04:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 via tagstart/tagend and 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.

Comment on lines 56 to 58
inputTags.forEach((inputTag) => {
if (!hasMatchingLabelTag(inputTag)) {
if (!inputTag.nested && !hasMatchingLabelTag(inputTag)) {
reporter.warn(
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread test/rules/input-requires-label.spec.js Outdated
Comment on lines 33 to 37
```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>
```
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

@ShivaniNR ShivaniNR Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed along with the header swap mentioned above.

Comment on lines 38 to 45
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'] })
}
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. labelDepth now only increments for labels without a for attribute. Labels with for go to labelTags for explicit matching only, so correctly warns.

@coliff
Copy link
Copy Markdown
Member

coliff commented Apr 17, 2026

<label><input type="password" /></label> isn't accessible since it doesn't include any actual text (label)...

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...

@coliff coliff marked this pull request as draft April 17, 2026 01:04
coliff and others added 4 commits April 17, 2026 22:27
- 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
Copy link
Copy Markdown
Author

<label><input type="password" /></label> isn't accessible since it doesn't include any actual text (label)...

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...

Good catch. I have updated the doc example to include visible text (<label>Password <input type="password" /></label>).

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.

@coliff coliff marked this pull request as ready for review April 18, 2026 00:47
@coliff
Copy link
Copy Markdown
Member

coliff commented Apr 18, 2026

Thanks for your work on this. Nice job!

Would it make sense to open a separate issue for that? Happy to work on it as a follow-up.

Yes please, that'd be great.

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.

input-requires-label rule should allow input tag nested inside label tag

3 participants