Repository navigation
fix(parser): don't treat an HTML void name in an svg or math namespace as void - #667
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
Follow-up to the namespace check for void names, from a review against Angular 22.2: - An open element with a void name is in a namespace (an HTML void element is never pushed), so Angular gives `:svg:area` the default tag definition: no child closes it and its parent's end tag can't skip it. `<svg><area><g></g></area></svg>` failed with `Unexpected closing tag "area"`, and `<svg><img><circle/></svg>` gave no error (Angular: `Unexpected closing tag ":svg:svg"`). - An i18n tag placeholder reads the parser's `is_void`: `<area>` in svg gets `START_TAG__SVG_AREA` and `CLOSE_TAG__SVG_AREA`. - The open elements are walked only for a void name (`is_html_void`). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95384184ba
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An open selectorless component is stored under its class name, so `<svg><MyComp:foreignObject><input></MyComp:foreignObject></svg>` missed the `foreignObject` boundary and read `input` as `:svg:input`. Like Angular's `_getPrefix`, which reads a component's `tagName`, the walk now uses the component's prefix and host tag (`ng-component` without one). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
<svg><area></area></svg>failed to compile withVoid elements do not have end tags "area", and<svg><area>{{ x }}</area></svg>lost the text insidearea. ngc 22.2 accepts both.Angular names an element without a prefix in its parent's namespace (
_getElementFullName), so<area>inside<svg>is:svg:area.getHtmlTagDefinition(':svg:area')is the default definition, which is not void. The HTML parser here looked the definition up by the local name only,area, both when it opened the element and when it checked the end tag.Changes
HtmlParser::inherits_namespacewalks the open elements from the closest one, skipping blocks. An element passes down its own prefix (svg:g), its implicit one (svg,math) or the one it took from its parent.foreignObjectpasses nothing down, aspreventNamespaceInheritancedoes upstream.consume_element_startonly marks an element void when it is in no namespace, so:svg:areakeeps its children and its end tag.consume_element_endonly reportsVoid elements do not have end tagsfor an element in no namespace.The namespaces themselves are still resolved after parsing, as before. With the fix, the full output for
<svg><area>{{ x }}</area><br></br><foreignObject><area><b>y</b></foreignObject></svg>has the same instructions as ngtsc 22.2.3 (withNO_ERRORS_SCHEMA, since ngtsc's schema check reports NG8001 for:svg:area).Not in this PR
<div/>gives noOnly void, custom and foreign elements can be self closederror here. Upstream marks every known DOM elementcanSelfClose: false(html_tags.ts, fromDomElementSchemaRegistry.allKnownElementNames()), andtags.rshas no such list, so every unknown name falls back tocanSelfClose: true. That is a separate gap.Checks
foreign_void_elementstests inhtml_parser_test.rs, each checked against ngc 22.2parseTemplate:<svg><area></area>,<svg><br></br>, a nested<g>,<math><br></br>, an@ifblock betweensvgandarea,<svg:area></svg:area>and<svg:g><area></area></svg:g>parse without errors;<svg><area>t</area>keeps the text;areainsideforeignObject, and at the top level, still reports the end tag.foreignObjectstop, the block skip, the explicit prefix check, or the namespace check at the start or at the end tag each fails a test.cargo test --all-features,cargo fmt --all -- --checkand conformance (1264/1264, snapshot unchanged) pass.Review follow-up (9538418, d96e722)
A sweep of ~220 templates against Angular 22.2 (HtmlParser tree and ngtsc output) found no regression. It did find places that still read
areaas void once the element is open:should_auto_close), and its parent's end tag can't skip it silently (closed_by_parent).is_void(html_to_r3.rs).is_html_void).<MyComp:foreignObject>,ng-componentwithout a tag), as Angular's_getPrefixreads a component'stagName.Not in this PR: other tag rules are still read by the local name inside svg/math. Main has the same behavior:
<svg><p><div></div></p>,<svg><li>a<li>b: closed by a child<svg><li>a</svg>: no error<svg><pre>\nx</pre>: first newline dropped<svg><pre><svg><p/>🤖 Generated with Claude Code