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

xmldom does not report parsererror (on XML without proper start tag) #349

Closed
edi9999 opened this issue Dec 13, 2021 · 4 comments · Fixed by #454
Closed

xmldom does not report parsererror (on XML without proper start tag) #349

edi9999 opened this issue Dec 13, 2021 · 4 comments · Fixed by #454
Labels
documentation Improvements or additions to documentation spec:HTML xml:not well-formed https://www.w3.org/TR/xml11/#dt-wellformed
Milestone

Comments

@edi9999
Copy link

edi9999 commented Dec 13, 2021

Compare this : (xmldom) :

> (new (require("@xmldom/xmldom").DOMParser)).parseFromString("a></a>", "text/xml").getElementsByTagName("parsererror")[0].textContent
Uncaught TypeError: Cannot read properties of undefined (reading 'textContent')

To this : (Firefox DOMParser) :

(new (DOMParser)).parseFromString("a></a>", "text/xml").getElementsByTagName("parsererror")[0].textContent
XML Parsing Error: syntax error
Location: about:newtab
Line Number 1, Column 1: newtab:1:1
"XML Parsing Error: syntax error
Location: about:newtab
Line Number 1, Column 1:a></a>

To this (Chrome DOMParser) :

(new (DOMParser)).parseFromString("a></a>", "text/xml").getElementsByTagName("parsererror")[0].textContent
'This page contains the following errors:error on line 1 at column 1: Document is empty\nBelow is a rendering of the page up to the first error.'

It seems that for the input "a>", xmldom says everythings ok will its not.

@karfau karfau added this to the before 1.0.0 milestone Dec 21, 2021
@karfau karfau added documentation Improvements or additions to documentation xml:not well-formed https://www.w3.org/TR/xml11/#dt-wellformed labels Dec 21, 2021
@karfau
Copy link
Member

karfau commented Dec 21, 2021

Thank you for reporting this.
Let me elaborate on the involved issues:

  1. xmldom currently never adds the parsererror element as specified in the HTML spec (but only applies to parsing XML, not HTML mime types.
  2. The specific snippet you provide has multiple issues when using it with xmldom:
    • it is not well-formed XML
    • xmldom doesn't detect it properly
    • the Document.documentElement is null when it's returned from parseFromString
    • the types provided by xmldom currently do not indicate that the documentElement could be null
  3. .getElementsByTagName("parsererror")[0] will only ever work with a document that has errors, the NodeList returned by getElementsByTagName has a length of 0, so accessing the first element returns undefined:

https://stackblitz.com/edit/js-xmldom349?devtoolsheight=33&file=index.js

Regarding 1. At some point I want to introduce the option to add this node, but it would be a massive change in behavior and implementation, so it' currently not really "planned".

Regarding 2. We currently prioritize issues with well formed or even valid XML hihger as those regarding non well formed XML. But the goal should be to at least report a fatalError if there is no documentElement after parsing. And when doing that, we could also add the parsererror tag just in that case.

Regarding 3. not going to be solved by this lib :)

@karfau karfau changed the title xmldom does not report parsererror on simple input xmldom does not report parsererror (on XML without proper start tag) Dec 21, 2021
@brodybits
Copy link
Member

I would like to chime in with a couple meta points:

  • @karfau added the documentation tag to imply that updating the documentation would likely be the primary or most important part
  • contributions of code & documentation updates from the community are always very welcome and encouraged for serious consideration (time permitting, of course but please feel free to follow up in case we drop the ball)

And thanks again to @karfau for essentially taking this project over!!

@karfau
Copy link
Member

karfau commented Dec 21, 2021

We label issues the explicitly want contributions with help-wanted and/or good first issue

Yes, documenting that would be helpful and contributions are always welcome, but I have not thought about where to best document this. We also have to document the current error handling in general (see #224), most likely this will happen prior to doing anything on this issue.
If somebody wants to contribute to this issue, please discuss the options here first.

@karfau karfau modified the milestone: before 1.0.0 Dec 21, 2021
@karfau karfau modified the milestones: before 1.0.0, 0.9.0 Oct 1, 2023
@karfau
Copy link
Member

karfau commented Oct 1, 2023

starting from https://github.com/xmldom/xmldom/releases/tag/0.9.0-beta.9 (available under @xmldom/xmldom@next), a ParseError is thrown when the documentElement is not present after parsing is done.
This behavior was introduced as part of #454

@karfau karfau closed this as completed Oct 1, 2023
@karfau karfau reopened this Oct 1, 2023
@karfau karfau linked a pull request Oct 1, 2023 that will close this issue
@karfau karfau closed this as completed Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation spec:HTML xml:not well-formed https://www.w3.org/TR/xml11/#dt-wellformed
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants