dperini / nwsapi

Fast CSS Selectors API Engine
MIT License
105 stars 37 forks source link

Throw when repeated :scope #33

Closed pkra closed 4 years ago

pkra commented 4 years ago

Basic info:

nwapi version: 2.2.0

Minimal reproduction case

Live example: https://jsbin.com/folowijuvi/edit?html,js,console,output

NW.Dom.install();
console.log(NW.Dom.select(':scope', document.body));
console.log(NW.Dom.select(':scope, :scope', document.body))
// []
// DOMException [SyntaxError]: unknown pseudo-class selector ':scope'
// emit@https://unpkg.com/nwsapi@2.2.0/src/nwsapi.js:565:17 
// compileSelector@https://unpkg.com/nwsapi@2.2.0/src/nwsapi.js:1292:21
// compile@https://unpkg.com/nwsapi@2.2.0/src/nwsapi.js:753:16
// collect@https://unpkg.com/nwsapi@2.2.0/src/nwsapi.js:1553:24
// _querySelectorAll@https://unpkg.com/nwsapi@2.2.0/src/nwsapi.js:1518:36

Notes

The real world use case was something like ':scope > span, :scope > div.

pkra commented 4 years ago

This seems to come down to

https://github.com/dperini/nwsapi/blob/3469ab37e85d0d686edd7938f87490a662472eb2/src/nwsapi.js#L1327

Changing the regular expression flag from i to ig would solve the problem but I don't know if that's too blunt.

I'm happy to make a PR if that helps.

pkra commented 4 years ago

The other scope tests still pass and test/scope/scope-03b.html seems like a good place to add a test for this (e.g., NW.Dom.select(':scope > .a1, :scope > .a2', aNode); should have two matches).

dperini commented 4 years ago

@pkra, thank you for your time and help fixing more exotic bugs.

Changing the regular expression flag from i to ig would solve the problem

that's what I also did as first shot fix and quick solution. It seems to be the right fix.

As you said the other ":scope" test are still working and I will also add the test you suggested.

There are other ":scope" related tests in the "test/w3c" folder if you want to check them out. To run those wpt tests it is necessary to first activate the embedded PHP server by running: sh w3c-tests.sh from inside the "test" folder. Then open http://localhost:8000 in your browser.

You will see a list of links to a series of wpt tests. The one containing extra ":scope" tests is the fourth link from the top named "Element Closest". On each of the tests you can press the CTRL+ALT keys combo to switch the URL and compare the results of the tests between nwsapi and native w3c Query Selectors API.

And again, thank you for contributing, help is always appreciated.

pkra commented 4 years ago

@dperini thanks for the response! I'll see if I can make a PR tomorrow and go through the tests more thoroughly -- just let me know if you'd prefer me not to at this point, whatever is easier for you.

pkra commented 4 years ago

Finally got around to this. I hope it's still of interest.

dperini commented 4 years ago

@pkra, sure it is still of interest and fixes the problem. Since I am short on time I would take advantage of your help and merge your fix. Thank you for contributing.

pkra commented 4 years ago

Thanks, @dperini!