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

v0.8.10 errors with "Cannot read properties of null (reading 'localName')" #583

Open
XhmikosR opened this issue Nov 22, 2023 · 5 comments
Open
Labels
bug Something isn't working needs investigation Information is missing and needs to be researched

Comments

@XhmikosR
Copy link

XhmikosR commented Nov 22, 2023

Describe the bug

Cannot read properties of null (reading 'localName')

Expected behavior

No errors

Runtime & Version:

xmldom version: > 0.8.8, e.g 0.8.10
runtime version: N/A
other related software and version: N/A

Additional context

We hit the issue in svg-sprite and thus in projects that depend on it. Example: https://github.com/twbs/icons/actions/runs/6935431138/job/18865536634#step:6:22

I had to pin the version to 0.8.8 in svg-sprite so that we don't get these errors. I didn't try v0.8.9, but I could give it a go later if it helps narrow down the problem.

@karfau
Copy link
Member

karfau commented Nov 23, 2023

It would be very helpful if you (or someone from svg-sprite) could provide a reproduction with

  • an xml/svg snippet that causes the issue
  • how exactly xmldom is used to reproduce the error

There usually is a link to a stackblitz template in the bug template which makes this very easy:
https://stackblitz.com/fork/js-xmldom-template?file=index.js

Thx

@XhmikosR XhmikosR changed the title versions > v0.8.8 errors with "Cannot read properties of null (reading 'localName')" v0.8.10 errors with "Cannot read properties of null (reading 'localName')" Nov 23, 2023
@XhmikosR
Copy link
Author

v0.8.9 also works for what is worth. So, it's the change in v0.8.10 that is a breaking change for us.

Unfortunately, I don't have a StackBlitz and the stacktrace is misleading (because it points to the svg-sprite source code). I'll see if I manage to create a StackBlitz with a reproducible example later.

@XhmikosR
Copy link
Author

XhmikosR commented Nov 23, 2023

Specifically, this change is the offending one: 0.8.9...0.8.10#diff-79db8c65cf49002eb2d5d7e81a17f3b73d94db6bb50c0c12f0c9f427aff4b11eL172

-return this[index] || null;
+return index >= 0 && index < this.length ? this[index] : null;

@karfau
Copy link
Member

karfau commented Nov 23, 2023

Any xml source and steps to be able to reproduce it are helpful. Could also be a dedicated small GitHub repo that i can clone to reproduce.
The more narrow the setup the better, but it's ok if it still involves svg-sprite

@karfau
Copy link
Member

karfau commented Nov 25, 2023

By the way, the change you point to, belongs to the only PR that is included in version 8.10.0:
#514
So most likely svg-sprite is trying to access an element after a DOM manipulation that no longer exists in the DOM.
Earlier versions didn't properly clean up in such cases, so that it was possible to access elements behind the index indicated by the updated length attribute.

And after reading the history of events in.the related issues an PRs, I checked if there is any difference in what is returned by Object.keys(element.childNodes)
in the different versions in question, and i can not see a difference in behavior:
https://stackblitz.com/edit/js-xmldom-template-b5bsdq?file=index.js,package.json
It is always ["0","1","length"]
But if course if you take the the keys in the beginning of the loop and iterate over them while they are modified and you keep iterating over them without checking that you get something back, it will not work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs investigation Information is missing and needs to be researched
Projects
None yet
Development

No branches or pull requests

2 participants