fix: compare tag and attribute names case insensitively - #1937
Open
arpitjain099 wants to merge 1 commit into
Open
arpitjain099 wants to merge 1 commit into
arpitjain099 wants to merge 1 commit into
Conversation
HTML tag and attribute names are ASCII case insensitive, and the parser hands them over exactly as they were written. Two default-on rules compare them raw, so the identical violation spelled in a different case is silently unreported. src-not-empty tests the tag with /^(img|script|embed|bgsound|iframe)$/, with no i flag, and the attribute with attr.name === 'src'. So <img src=""> is reported and <img SRC="">, <IMG src=""> and <IMG SRC=""> are not. The same applies to link/href and object/data. attr-no-duplication keys its seen-set on the raw attribute name, so <div id="a" id="b"> is reported and <div ID="a" id="b"> is not. 28 of the 44 rules already normalise with toLowerCase; this brings these two into line. The reported message still echoes the attribute as the author wrote it. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Use ASCII-only normalization and preserve the original tag spelling in diagnostics.
Pull request overview
Updates HTMLHint rules to compare tag and attribute names case-insensitively, with regression tests and regenerated distribution files.
Changes:
- Normalizes names in
src-not-emptyandattr-no-duplication. - Adds mixed-case regression tests.
- Regenerates compiled JavaScript.
- Requires ASCII-only normalization and preservation of original diagnostic spelling.
File summaries
| File | Summary |
|---|---|
test/rules/src-not-empty.spec.js |
Tests case-insensitive source checks |
test/rules/attr-no-duplication.spec.js |
Tests case-insensitive duplicate detection |
src/core/rules/src-not-empty.ts |
Case-insensitive tag and attribute matching |
src/core/rules/attr-no-duplication.ts |
Case-insensitive duplicate detection |
dist/core/rules/src-not-empty.js |
Regenerated rule output |
dist/core/rules/attr-no-duplication.js |
Regenerated rule output |
Review details
Suppressed comments (3)
src/core/rules/attr-no-duplication.ts:19
String.prototype.toLowerCase()performs Unicode case mappings, but HTML attribute names are only ASCII case-insensitive. For example, the distinct attribute names U+0130 (Latin capital I with dot) andifollowed by U+0307 both normalize to the latter here and will be reported as duplicates. Fold onlyA–Z(and add a regression case) so non-ASCII attribute names remain distinct.
attrName = attr.name.toLowerCase()
src/core/rules/src-not-empty.ts:11
String#toLowerCase()applies Unicode case folding, but HTML name matching is ASCII case-insensitive. This can cause false positives for non-ASCII names (for example, JavaScript lowercasesKtok, so a<div k="a" K="b">pair can be reported as duplicate, and Unicode characters such asſcan make a non-HTML tag match a built-in tag). Use an ASCII-only A–Z normalization for bothtagNameandattrName(and add a regression case) so only ASCII case variants compare equal.
const tagName = event.tagName.toLowerCase()
src/core/rules/src-not-empty.ts:11
- The normalized
tagNameis also interpolated into the existing diagnostic, so uppercase markup now reportstag [ img ]instead of preserving the source spelling (tag [ IMG ]). Keep the normalized value for comparisons but useevent.tagNamewhen constructing the message, matching the existing preservation ofattr.name.
const tagName = event.tagName.toLowerCase()
- Files reviewed: 4/6 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
HTML tag and attribute names are ASCII case insensitive, and the parser passes them through exactly as written. Two rules in
defaultRulesetcompare them raw, so the same violation in a different case goes unreported.src-not-emptymatches the tag with noiflag and the attribute by exact string:attr-no-duplicationkeys its seen-set onattr.namedirectly.Four spellings of one bug, plus the duplicate-attribute pair, through the CLI on
main. The two casing rules are turned off in the config, which is routine: SVG'sviewBox, Angular and Vue attribute names and generated markup all trip them legitimately.Lines 6, 7 and 8 are
<img SRC="">,<IMG src="">and<IMG SRC="">. None of them draws anything. Had line 5 not been in the file it would have exited clean.After the change the same file reports 7, with all four
src-not-emptycases and bothattr-no-duplicationcases:The message still echoes the attribute as the author spelled it, so the output points at the real text.
This is not a new convention for the project:
grep -L toLowerCase src/core/rules/*.tsshows 28 of the 44 rules already normalise, and these two were among the 16 that did not.Verification
Three cases added: the four
img/srcspellings, uppercaseLINK HREFandOBJECT DATA, andhref/HREFduplicated on one element. All three fail onmain.npm testgoes from 370 to 373 passing with no failures,npm run lintreports no issues andprettier --checkis clean on all four touched files.dist/is regenerated and included, since a clean build onmainproduces no churn there and the test suite imports../../dist/htmlhint.js. If you would rather that be left out of contributions, say so and I will drop it.