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

Error when using root option in jsdom #65

Closed
Treora opened this issue Jun 3, 2021 · 7 comments
Closed

Error when using root option in jsdom #65

Treora opened this issue Jun 3, 2021 · 7 comments
Assignees
Labels

Comments

@Treora
Copy link

Treora commented Jun 3, 2021

In the test case below, things work fine until I supply the root option. I have not tried whether the same problem occurs in a browser.

self = globalThis; // css-selector-generator fails without `self` — separate issue?
cssSelectorGenerator = require('css-selector-generator').default
jsdom = require('jsdom').JSDOM;

html = '<b>lorem <i>ipsum</i> dolor <u><i>amet</i> yada <i>yada</i></u></b>';
doc = (new jsdom(html)).window.document;
target = doc.querySelector('u i');
root = doc.querySelector('u');

cssSelectorGenerator(target, { root });

This fails with:

Uncaught DOMException [SyntaxError]: '> i' is not a valid selector

Also, when using some other elements as root, it returns without errors, but the root seems to have been ignored:

b = doc.querySelector('b')
cssSelectorGenerator(target, { root: b });
cssSelectorGenerator(target);

Both of the above return the same value: ':root > :nth-child(2) > :nth-child(1) > :nth-child(2) > :nth-child(1)'

Treora added a commit to apache/incubator-annotator that referenced this issue Jun 3, 2021
I tried a few css selector generators, listed here:
<https://github.com/fczbkk/css-selector-generator-benchmark>

- css-selector-generator failed when a root (= scope) is passed; see
  issue <fczbkk/css-selector-generator#65>.

- using @mdev/finder instead gave syntax errors due to ‘export’ token.
  (perhaps because we don’t transpile dependencies; worth considering?)

- optimal-select seemed to work; whatever works is good enough for now.

I made describeCss accept an Element, not a Range, for its scope and
target, as Ranges make little sense for a CssSelector; I figured we may
want to change this in the matcher too, and perhaps more widely.
@fczbkk fczbkk self-assigned this Jun 6, 2021
@lilacdev
Copy link

lilacdev commented Aug 18, 2021

I can confirm this behaviour with Chrome. What my tests found when a root element is provided :

  • If it is impossible to find a proper selector (with too restrictive selectors option for example), I have a "Failed to execute 'querySelectorAll' on 'Element': '> li' is not a valid selector"
  • if it is possible to find a selector, result fallback to an absolute selector and root element is ignored.

@fczbkk
Copy link
Owner

fczbkk commented Aug 18, 2021

This issue was not specific to JsDom. It was caused by attempting to test a descendant selector candidate which is direct descendant of root element (which itself has no selector).

It should be fixed now. Please update to v3.2.1.

@fczbkk fczbkk closed this as completed Aug 18, 2021
@fczbkk
Copy link
Owner

fczbkk commented Aug 18, 2021

@Treora Regarding other problems you mentioned in your report:

The window.self probably does not exist in the environment that you are running your code in (I guess it's Node). This is not the issue of CssSelectorGenerator library, which targets browsers only.

The fallback selector ignores the root defined in the options, because that is the only way it can produce reliable and unique selector. It needs to start with the :root selector. That's because the entire hierarchy of document counts, even if you call Element.querySelector() from specific element.

@fczbkk
Copy link
Owner

fczbkk commented Aug 18, 2021

@Treora By the way, check out these instructions on how to add external code (e.g. library) to JsDom. I think it will help you with issues like missing global properties (e.g. window.self).
https://github.com/jsdom/jsdom/wiki/Don't-stuff-jsdom-globals-onto-the-Node-global

@lilacdev
Copy link

Hi @fczbkk ,

Still have issue. I checked versions, and it appears that we are in 3.4.0 and not 3.2.1. Is this ok ?

I forced update anyway, and issue still occurs. You can test it here with those elements :
http://histo.io
root element is first occurence of nav > ul > li > ul .
Then trying to get selector for any chiled a element, but it returns full path (I use blacklist ['[href=*]', '[src=*]'].

@fczbkk
Copy link
Owner

fczbkk commented Aug 24, 2021

@lilacdev Can you please open a standalone issue for this and include specific instructions how to replicate this problem? Even better, if you could create a minimal isolated test case. Thank you.

@lilacdev
Copy link

Of course, I was writing piece of code to reproduce it :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants