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

:scope pseudo-class fails for some XML documents with HTML-incompatible id #53

Closed
mardanbeigi opened this issue Feb 2, 2022 · 3 comments

Comments

@mardanbeigi
Copy link

mardanbeigi commented Feb 2, 2022

:scope fails to work with XML documents which have HTML-incompatible ids. Example:

doc = new DOMParser().parseFromString(`<?xml version="1.0" encoding="UTF-8"?><outer id='1a'><inner id='a'><inner></inner></inner></outer>`, "text/xml")

doc.firstChild.querySelectorAll(':scope > inner')  # WAI

This works on Chrome. However, using nwsapi fails:

NW.Dom.select(':scope inner', doc.firstChild)   # fails

The failure stems from :scope pseudo-class being replaced with element references which then create a selector that is invalid since HTML id attributes can't start with numbers: Uncaught DOMException: 'outer#1a inner' is not a valid selector

edit: hit enter too fast

@b-fuze
Copy link

b-fuze commented May 12, 2022

We have a related issue regarding scope in b-fuze/deno-dom#97. We have the following code that returns the correct output in the browser, but incorrect output when used with nwsapi.

const doc = new DOMParser().parseFromString(
  `
    <div>
      <div class=outer>
        <div class=inner></div>
      </div>
      <div class=other-outer></div>
    </div>
  `,
  "text/html",
);

const parent = doc.querySelector("div");
console.log(
  [...parent.querySelectorAll(":scope > div")].map((d) => d.className),
);

Outputs the following with in the browser:

[ "outer", "other-outer" ]

Whereas it outputs the following with nwsapi

[ "outer", "inner", "other-outer" ]

Which is also due to the fact that the :scope selector in nwsapi isn't aware of the actual context, as makeref tries to make a reference to the element (<div>) but there's nothing noteworthy to identify it with so it matches all <div>s

@dperini
Copy link
Owner

dperini commented Apr 6, 2023

@b-fuze current release of nwsapi produces correct output as you outlined above.
@mardanbeigi nowaday browsers and nwsapi accept id starting with digits...
I am closing this because I can't reproduce issues, works as expected.

@dperini dperini closed this as completed Apr 6, 2023
@mardanbeigi
Copy link
Author

mardanbeigi commented Apr 10, 2023

I'm still seeing nwsapi fail on the leading digit, with a fresh clone of the project:

doc = new DOMParser().parseFromString(`<?xml version="1.0" encoding="UTF-8"?><outer id='1a'><inner id='a'><inner></inner></inner></outer>`, "text/xml")
NW.Dom.select(':scope inner', doc.firstChild)

error:

Uncaught DOMException: 'outer#1a inner' is not a valid selector
emit | @ | nwsapi.js:559
_querySelectorAll | @ | nwsapi.js:1504

Likely due to this identifier regex that disallows leading digits: https://github.com/dperini/nwsapi/blob/master/src/nwsapi.js#L596

edit: fix formatting

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

No branches or pull requests

3 participants