Fix: Whitespace characters will incorrectly trigger foster parenting.#31
Fix: Whitespace characters will incorrectly trigger foster parenting.#31zimya wants to merge 3 commits into
Conversation
|
Hi! It seems you removed the template which we require. Here are our templates (pick the one you want to use and click *raw* to see its source): I won’t send you any further notifications about this, but I’ll keep on updating this comment, and hide it when done! Thanks, |
|
Could you add a test for this? |
There was a problem hiding this comment.
Pull request overview
This PR addresses incorrect HTML5 foster parenting triggered by structural whitespace within table-related insertion modes when feeding tokens into parse5, improving table structure preservation when running a HAST tree through hast-util-raw/rehype-raw.
Changes:
- Classify whitespace-only text nodes as
Token.TokenType.WHITESPACE_CHARACTERinstead of alwaysCHARACTER. - Make namespace detection in
startTag()resilient when there is no current open element. - Minor formatting/indentation adjustment in
documentMode()(no functional change).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const isWhitespace = /^[ \t\n\f\r]+$/.test(node.value) | ||
| const tokenType = isWhitespace | ||
| ? Token.TokenType.WHITESPACE_CHARACTER | ||
| : Token.TokenType.CHARACTER | ||
|
|
||
| /** @type {Token.CharacterToken} */ | ||
| const token = { | ||
| type: Token.TokenType.CHARACTER, | ||
| type: tokenType, | ||
| chars: node.value, |
There was a problem hiding this comment.
Please add a regression test for this new whitespace-token behavior (e.g., a <table> with \n / formatting-only text nodes between table elements) to ensure raw() no longer causes parse5 foster parenting to move those text nodes outside the table.
| const isWhitespace = /^[ \t\n\f\r]+$/.test(node.value) | ||
| const tokenType = isWhitespace |
There was a problem hiding this comment.
The /^[ \t\n\f\r]+$/ regex literal is evaluated each time text() runs, creating a new RegExp object per text node. Consider hoisting it to module scope (or reusing a shared constant) to avoid unnecessary allocations in large documents.
|
Thank you for the review! I've hoisted the RegExp to the module scope as whitespaceExpression to avoid unnecessary allocations, and added a regression test checking that whitespace text nodes within tables are kept inside the table and not foster-parented. |
Initial checklist
Description of changes
Fixes #30
This PR fixes an issue where structural whitespace (like newlines) inside table elements gets incorrectly foster-parented outside the table when passing a hast tree through
rehype-raw.Currently,
hast-util-rawunconditionally maps all text nodes toToken.TokenType.CHARACTERwhen feeding tokens toparse5. Whenparse5receives aCHARACTERtoken inside restricted table modes (e.g.,in table body,in row), it treats it as illegal text and triggers HTML5 Foster Parenting, extracting the newlines and flattening the table.This PR added a regex check
(/^[ \t\n\f\r]+$/)in thetext()handler. If the text node contains only HTML whitespace characters, it is correctly mapped toToken.TokenType.WHITESPACE_CHARACTER. This allowsparse5to safely ignore/append the formatting whitespace without breaking the table structure.