Jsdom: querySelectorAll doesn't support namespaced attributes

Created on 26 Oct 2017 · 22Comments · Source: jsdom/jsdom

Selectors targetting namespaced attributes (for instance, [*|href]) raise a SyntaxError.

Basic info:

Node.js version: v8.7.0
jsdom version: v11.3.0

Minimal reproduction case

const { JSDOM } = require("jsdom");

const dom = new JSDOM('<!doctype html><svg xlink:href="foo"/>');

dom.window.document.querySelectorAll('[*|href]');

throws:

    SyntaxError: The string "[*|href]", is not a valid CSS selector

although the selector is valid.

How does similar code behave in browsers?

It selects the svg element. See http://jsbin.com/relisatula/edit?html,console

See also #1587 (although I think this one is slightly different, as it not only affects XML documents but also HTML documents).

maybe fixed (add tests to confirm) selectors

Source

rdeltour

Most helpful comment

@domenic / @rdeltour ,
the new "nwsapi" has support for this, the namespace grammar is supported in the validator and parser for both attributes and tag names, I could not test the results but at least no error should be thrown. The addition was necessary to pass the complete w3c test suite.

Please see here: https://github.com/dperini/nwsapi

I have not talked/publicized much the new "nwsapi", but it is ready, so I take the chance here ...

"nwsapi" is the new Selectors Level 4 API emulation library, the continuation of my "nwmatcher" project. It targets newer browsers and headless environments, it has support for most of the new additions in the Selectors Level 4 specification, namely:

L4 web-form validation related pseudo-classes
L4 dynamic and user action pseudo-classes
namespace grammar and elements resolver
throwing real DOMException SyntaxError
native closest method on elements
:scope reference pseudo-class
:matches(s1, s2) pseudo-class
:not(s1, s2) negation pseudo-class
install/uninstall methods to replace QSA
modular design allows future L4 extension
only 24Kb minified, 8Kb minified & compressed

I am especially happy about the unbelievable speed improvements, have a peek yourself here:

http://javascript.nwbox.com/nwsapi/test/speed/

the biggest improvements in speed have been implemented on the most weak areas of "nwmatcher" and other libraries, for example on comma-separated groups, the :not() pseudo-class and on tree-structural pseudo-classes (:root, nth child-indexed and typed child-indexed).

The comparison in the above speed test is against the latest version of "nwmatcher" (1.4.3) and the native browsers QSA, there is no game in comparing with other CSS Selectors library, for most selectors "nwsapi" already outruns native QSA implementations and I can still see there is space for improvements.

It would be nice to have your feedback after running the above speed benchmark, I used Sizzle own Selectors Test Suite as a start but rewrote parts of it. I need to do more work on that.

I also included the latest w3c/web-platform-tests in the GitHub release in the test directory.
Everything passes in the new and old testing I have, a few corrections and it will be ready for npm.

dperini on 26 Oct 2017

👍3 🎉2

All 22 comments

@dperini interesting nwmatcher feature request

domenic on 26 Oct 2017

Please see here: https://github.com/dperini/nwsapi

I have not talked/publicized much the new "nwsapi", but it is ready, so I take the chance here ...

L4 web-form validation related pseudo-classes
L4 dynamic and user action pseudo-classes
namespace grammar and elements resolver
throwing real DOMException SyntaxError
native closest method on elements
:scope reference pseudo-class
:matches(s1, s2) pseudo-class
:not(s1, s2) negation pseudo-class
install/uninstall methods to replace QSA
modular design allows future L4 extension
only 24Kb minified, 8Kb minified & compressed

I am especially happy about the unbelievable speed improvements, have a peek yourself here:

http://javascript.nwbox.com/nwsapi/test/speed/

It would be nice to have your feedback after running the above speed benchmark, I used Sizzle own Selectors Test Suite as a start but rewrote parts of it. I need to do more work on that.

I also included the latest w3c/web-platform-tests in the GitHub release in the test directory.
Everything passes in the new and old testing I have, a few corrections and it will be ready for npm.

dperini on 26 Oct 2017

👍3 🎉2

Ow wow, that's so exciting!!! This will make people really happy, and be great for jsdom. I'll definitely try to find some time to play around with it and give feedback.

domenic on 26 Oct 2017

👍1

@dperini Those speed improvements are incredible - not to mention all the features you've added! Fantastic work!

Zirro on 27 Oct 2017

@domenic @Zirro @rdeltour thank you for sharing the excitement and for the support ...
I hope you can test "nwsapi" in "jsdom" environments and report about problems/inconsistencies.
Notice that the version on GitHub is 5/6 commits behind my local WIP that you are testing at:

http://javascript.nwbox.com/nwsapi/test/speed/

The reason is that I am still improving the nth/of-type and comma-separated resolvers to squeeze out every drop of speed enhancements. So the on-line speed test is what will be the real "nwsapi" in a few more days as soon as I commit the rest of the WIP.

I am very happy about the speed enhancements, the 2x/4x figures I predicted were met and probably also under-estimated in some area, however be assured there is no trick, no caching of results and no use of native QSA.

The gains comes from a really good RTL strategy (inherited from "nwmatcher") which allows me to avoid having to sort, reorder elements and check for uniqueness, they are in fact already document ordered and unique since the beginning of the elements collection phase using the bare-bone gEBTN dom API or an efficient recursive RTL walk where gEBTN is not available, also not having to support detrimental IE browsers helps with speed and simplicity. Traversal API helps too.

Hope it works out of the box for jsdom, i did some cursory tests but will wait for feedback in case some fixes and tweaks were necessary. At the moment "nwsapi" passes all the tests at 100% including w3c latest Selectors Suite, throws the same errors/exceptions like the real QSA, it is easily auto installed as a full emulation and replacement, support extra callback on results and ...

dperini on 27 Oct 2017

Looking forward to that update, right now I have a lot of unit tests that I cannot run because of all the :scope query selectors. Any ETA?

rraziel on 25 Nov 2017

@rraziel the :scope pseudo-class is quite new and I could do just minor testing.
Do you think you could use the current "nwsapi" as is to run your tests ?
Even running your unit tests in a normal browser would be enough.
It could be of great help and also speed up the release process.
If :scope is working I would try to port it to "nwmatcher".

dperini on 26 Nov 2017

@dperini I can try to help, although I'm not quite sure how I would go about testing it directly (I'm using jest, which eventually uses jsdom which uses nwmatcher, from what I understand).
Or you'd just like me to write tests for it in the nwsapi project?

rraziel on 26 Nov 2017

@rraziel, if you have the time, writing tests for "nwsapi" would also help, as you can see I only have a couple of test for the ':scope' pseudo-class and they do not cover enough cases.
Maybe @domenic or @Zirro can help us here by providing a copy/fork of "jsdom" using "nwsapi" instead of "nwmatcher". That would help in transitioning from "nwmatcher" to "nwsapi".
If necessary I can already publish "nwsapi" on "npm".

dperini on 26 Nov 2017

This is still not working, although this time I think the problem is more likely to be in the jsdom layer, not the nwsapi layer. The selector no longer throws, but it doesn't match anything.

Test:

<!DOCTYPE html>
<meta charset="utf-8">
<title>querySelectorAll must work with namespace attribute selectors on SVG</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<!-- Regression test for https://github.com/jsdom/jsdom/issues/2028 -->

<svg id="thesvg" xlink:href="foo"></svg>

<script>
"use strict";
const el = document.getElementById("thesvg");

assert_array_equals(document.querySelectorAll("[*|href]"), [el]);

done();
</script>

domenic on 16 Mar 2019

@domenic, @zirro,
in this case the reported issue is not due to jsdom but to nwsapi.
I just pushed a commit as an attempt to fix this in my nwsapi repository.
Care to try it out ?

dperini on 19 Mar 2019

👍1

@dperini Nice! It fixes this issue, though it created some new failures in the WPT dom/nodes ParentNode-querySelector-All-xht.xht with the selector [*|TiTlE] that appear to be related to case-sensitivity. [*|TiTlE] should not match <div title=""></div>.

Zirro on 20 Mar 2019

@zirro, yes you are correct ...
I forgot to make it case-insensitive !
Will fix this asap, thank you for testing it.

dperini on 21 Mar 2019

@dperini I tried the test again with the latest [email protected], but it still fails. Any ideas?

domenic on 21 Apr 2019

Can you test with the latest code in my "nwsapi" repository.
The latest commits should have solved this specific issue.
I am ready to publish that on npm as 2.1.4 if you confirm.

dperini on 22 Apr 2019

Pushed new release 2.1.4 of 'nwsapi' on NPM.
Should fix this issue.

dperini on 30 Apr 2019

The test in https://github.com/jsdom/jsdom/issues/2028#issuecomment-473572499 still fails. I have posted it in a branch at https://github.com/jsdom/jsdom/tree/test-2028 if that makes testing easier.

domenic on 28 May 2019

@domenic
this issue should be resolved by these two commits:
https://github.com/dperini/nwsapi/commit/f6509bcd434f7438ec1599430c579289928d6a77
https://github.com/dperini/nwsapi/commit/be1d1a1c710847a95467bb2119d6e78c35bbdc02
It is ready for publishing but due to other changes I believe it deserve 2.2.0 version.
Waiting for your review and agreement before publishing on npm.

dperini on 31 May 2019

@dperini confirmed those commits fix the issue (and don't cause any new regressions). When you have time, a new release would be great!

domenic on 19 Oct 2019

@domenic A new release is on the way, it may be next weekend I have the time for it.
Thank you for reviewing the two commits above.

dperini on 22 Oct 2019

@domenic @dperini there is still an issue with this. I have a case were there are backslashes in the attribute name. When I replace them with normal slashes it finds the node. Otherwise it fails.
I tried this in a non-namespaced context and it worked out fine