fix: removeAttr case-insensitive for HTML documents - #5313
Conversation
HTML attribute names are case-insensitive and htmlparser2 stores them in lowercase. removeAttr() now lowercases the attribute name when not in xmlMode, matching the behavior of attr() and the HTML spec. XML mode preserves case sensitivity as expected.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03e1f0eaab
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (this.options.xmlMode) { | ||
| removeAttribute(elem, attrName); | ||
| } else { | ||
| removeAttribute(elem, attrName.toLowerCase()); |
There was a problem hiding this comment.
In an HTML document that contains SVG, parse5 stores adjusted foreign-content attributes such as viewBox under their canonical camel-case names, so $('svg').attr('viewBox') can read them. This branch lowercases every name whenever xmlMode is false, so $('svg').removeAttr('viewBox') tries to delete viewbox and leaves the original viewBox attribute in place. Please only apply case-folding to HTML elements, or otherwise remove the actual matching key case-insensitively for HTML attributes.
Useful? React with 👍 / 👎.
…eriojs#5073) When an HTML boolean attribute has a value (e.g. hidden="until-found"), .attr() now returns the actual value instead of normalizing to the attribute name. Pure boolean attributes (empty value) still return the attribute name. Fixes cheeriojs#5073
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b005bf4522
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!xmlMode && rboolean.test(name) && value === '') { | ||
| return name; | ||
| } | ||
| return value; |
There was a problem hiding this comment.
When an HTML boolean attribute is present with a non-empty value such as disabled="false" or readonly="custom", this now returns the literal value instead of the attribute name. That regresses the existing Cheerio/jQuery-style contract documented above this helper and used by the existing boolean-attribute tests: these attributes are true by presence, so callers using .attr('disabled') to detect presence now see misleading values even though .prop('disabled') remains true. Please limit the value-preserving special case to hidden="until-found" (or remove hidden from the boolean set) rather than changing all boolean attributes.
Useful? React with 👍 / 👎.
|
Thanks! Duplicate of #5290. |
2 participants
Problem (#5257)
.removeAttr()did not lowercase the attribute name before removing it fromelem.attribs. Since htmlparser2 stores all attribute names in lowercase (per HTML spec), calling.removeAttr("CLASS")would fail to remove aclassattribute.Fix
Lowercase the attribute name when not in
xmlMode, matching the behavior ofattr()and the HTML spec. XML mode preserves case sensitivity.Testing