Skip to content

fix: drop whitespace outside the root element instead of saving it as a text node - #917

Open
yxshee wants to merge 1 commit into
NaturalIntelligence:masterfrom
yxshee:fix/prolog-whitespace-text-node
Open

yxshee wants to merge 1 commit into
NaturalIntelligence:masterfrom
yxshee:fix/prolog-whitespace-text-node

Conversation

@yxshee

@yxshee yxshee commented Nov 9, 2026

Copy link
Copy Markdown

Purpose / Goal

With trimValues: false, the whitespace between the XML declaration (or the DOCTYPE) and a comment or processing instruction is saved as a text node of the document itself, while the same whitespace before the root element is dropped.

Input

const xml = `<?xml version="1.0"?>
<!-- c -->
<root/>
`;
new XMLParser({ preserveOrder: true, commentPropName: "#comment", trimValues: false }).parse(xml);
new XMLParser({ commentPropName: "#comment", trimValues: false }).parse(xml);

Actual output

[ { "?xml": [ { "#text": "" } ] }, { "#text": "\n" }, { "#comment": [ { "#text": " c " } ] }, { "root": [] } ]
{ "?xml": "", "#comment": " c ", "root": "", "#text": "\n" }

Expected output

[ { "?xml": [ { "#text": "" } ] }, { "#comment": [ { "#text": " c " } ] }, { "root": [] } ]
{ "?xml": "", "#comment": " c ", "root": "" }

Whitespace outside the root element is never part of the document (XML 1.0, production [27] Misc), and the parser already drops it when the next thing it meets is the root element: the opening-tag branch skips text whose parent is the document node (currentNode.tagname !== '!xml'), but the comment, PI, CDATA and closing-tag branches call saveTextToParentTag unconditionally. With preserveOrder the stray {"#text": "\n"} shifts every following index, and without it the result object gets a "#text" property next to the root element. The same happens for whitespace after the root element when it is followed by a comment.

Found by comparing the parser's output with xmldom on generated, libxml2-validated documents; this was the only divergence left once the documented ones (numeric character references, attribute value normalization) were set aside.

Fix

saveTextToParentTag now returns without saving when the parent is the document node and the text is whitespace only. Non-whitespace text outside the root element is still saved as before, so the lenient handling of malformed input from #861 (</a>text</b> → {"#text": "text"}) is unchanged, and trimValues: true is unaffected because it already trimmed that text to nothing.

The spec covers the declaration, a comment and a PI in the prolog and a comment after the root element, in both preserveOrder and plain mode, and checks the whitespace inside the root element is still kept.

Performance

Parsing spec/assets/midsize.xml 20 times with trimValues: false (best of 5 rounds, three runs each, Node 26): 9184 / 9519 / 9458 ms before and 9763 / 8998 / 9271 ms after, so the extra check is within the run-to-run noise. (node benchmark/XmlParser.mjs needs benchmark, xml2js and xmlbuilder2, which are not in the dev dependencies.)

Type

  • Bug Fix
  • Refactoring / Technology upgrade
  • New Feature

… a text node

With trimValues: false, the whitespace between the XML declaration (or the
DOCTYPE) and a comment or processing instruction was saved as a text node of
the document itself: `[{"#text": "\n"}, ...]` with preserveOrder, and a
`"#text": "\n"` property next to the root element without it. The same
whitespace before the root element was already dropped, because the
opening-tag branch skips text whose parent is the document node while the
comment, PI, CDATA and closing-tag branches save it unconditionally.

Whitespace outside the root element is never part of the document (XML 1.0
production [27] Misc), so saveTextToParentTag now drops whitespace-only text
whose parent is the document node. Non-whitespace text outside the root is
still saved as before, so the lenient handling of malformed input such as
`</a>text</b>` is unchanged.

This branch has not been deployed

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

1 participant