Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trailing whitespace in processing instructions not preserved #497

Closed
shunkica opened this issue Jun 13, 2023 · 2 comments · Fixed by #498
Closed

Trailing whitespace in processing instructions not preserved #497

shunkica opened this issue Jun 13, 2023 · 2 comments · Fixed by #498
Labels
bug Something isn't working good first issue Good for newcomers help-wanted External contributions welcome spec:DOM-Level-2 https://www.w3.org/TR/DOM-Level-2-Core/ xml:well-formed https://www.w3.org/TR/xml11/#dt-wellformed

Comments

@shunkica
Copy link
Collaborator

shunkica commented Jun 13, 2023

Describe the bug
Trailing whitespace in PI is not preserved.
One trailing whitespace is preserved if the PI does not have data.

To Reproduce
https://stackblitz.com/edit/js-xmldom-template-mcmkty?file=index.js

Expected behavior
The PI without data should have no trailing whitespace.
The trailing whitespace in PI with data should be preserved.
From DOM Level 1 spec:

Attributes
target
The target of this processing instruction. XML defines this as being the first token following the markup that begins the processing instruction.
data
The content of this processing instruction. This is from the first non white space character after the target to the character immediately preceding the ?>.

Runtime & Version:
xmldom version: 0.8.8

@shunkica shunkica added bug Something isn't working needs investigation Information is missing and needs to be researched labels Jun 13, 2023
@shunkica shunkica changed the title Trailing whitespace in PI not preserved Trailing whitespace in processing instructions not preserved Jun 13, 2023
@karfau karfau added good first issue Good for newcomers help-wanted External contributions welcome spec:DOM-Level-2 https://www.w3.org/TR/DOM-Level-2-Core/ xml:well-formed https://www.w3.org/TR/xml11/#dt-wellformed and removed needs investigation Information is missing and needs to be researched labels Jun 14, 2023
@karfau
Copy link
Member

karfau commented Jun 14, 2023

Thank you for reporting the issue.
I think the culprit is in the regular expression in https://github.com/xmldom/xmldom/blob/master/lib/sax.js#L611, which seems to drop whitespace before the closing symbols.

Here is the same section in the DOM Level 2 spec: https://www.w3.org/TR/DOM-Level-2-Core/#core-ID-1004215813

The XML spec is not really saying anything about the data, only about the target: https://www.w3.org/TR/2006/REC-xml11-20060816/#sec-pi

@karfau
Copy link
Member

karfau commented Jun 14, 2023

If I have to tackle it myself it will happen after I released 0.9.0, so there will be a patch for 0.8.x but not for 0.7.x

But if somebody else takes care of it before 0.9.0 is released 0.7.x will also be patched.

@karfau karfau linked a pull request Jul 3, 2023 that will close this issue
karfau added a commit that referenced this issue Jul 10, 2023
Add support for parsing the internal subset of a DOCTYPE and saving it
as a string in `DocumentType.internalSubset`.

BREAKING CHANGE: Many documents that were previously accepted by xmldom,
esecially non well-formed ones are no longer accepted. Some issues that
were formerly reported as `error`s are now a `fatalError`.

fixes #117, #497 

Spec: [XML DOM L2
Core](https://www.w3.org/TR/DOM-Level-2-Core/#core-ID-412266927)

---------

Co-authored-by: Ivan <admin@clab.hr>
Co-authored-by: Christian Bewernitz <coder@karfau.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help-wanted External contributions welcome spec:DOM-Level-2 https://www.w3.org/TR/DOM-Level-2-Core/ xml:well-formed https://www.w3.org/TR/xml11/#dt-wellformed
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants