capricorn86 / happy-dom

A JavaScript implementation of a web browser without its graphical user interface
MIT License
3.1k stars 185 forks source link

`querySelectorAll` finds indirect children when `selector` starts with `>` #564

Open svdgriendt opened 1 year ago

svdgriendt commented 1 year ago

The following can be tested directly on https://npm.runkit.com/happy-dom There I entered the following

var happyDom = require("happy-dom");

const window = new happyDom.Window();
const document = window.document;
document.body.innerHTML = `<div class="outer">
  <div class="inner">
    <div class="inner"></div>
  </div>
  <div class="other"</div>
</div>`;

const outer = document.querySelector('.outer');
const innerAll = outer.querySelectorAll('.inner');
const innerAllScoped = outer.querySelectorAll(':scope .inner');
const innerDirect1 = outer.querySelectorAll('> .inner');
const innerDirect2 = outer.querySelectorAll(':scope > .inner');

console.log('all', innerAll.length);              // prints 2
console.log('all scoped', innerAllScoped.length); // prints 0, expected 2
console.log('direct 1', innerDirect1.length);     // prints 2, expected 1 or error
console.log('direct 2', innerDirect2.length);     // prints 0, expected 1

If I run the same in a browser, innerDirect1 will result in an error, because it's incorrect syntax. I've included what the expected values are, so that they match with the 'normal' querySelectorAll. Now I don't really care for :scope, but I do care for >, because I'm writing tests that need this to work as expected.

I managed to find one issue that may be related to this, but it's actually quite old and already resolved: #143 And I'm using 6.0.4 by the way.

svdgriendt commented 1 year ago

I've dug a bit into the code, and I found the behavior in https://github.com/capricorn86/happy-dom/blob/8e2723cb0a996e0e7f3dbe40e7c1d7d4d74b2fa1/packages/happy-dom/src/query-selector/QuerySelector.ts#L164-L202 to be a bit weird. When selector starts with > (or , for that matter), then currentSelector is always pushed into parts. The effect of this, is that with selector = '> .inner', the resulting parts is ['', '>', '.inner'].

Now I looked a bit further, and noticed that https://github.com/capricorn86/happy-dom/blob/8e2723cb0a996e0e7f3dbe40e7c1d7d4d74b2fa1/packages/happy-dom/src/query-selector/QuerySelector.ts#L69-L103 has a check if the first element in selectorParts equals >. That's not the case for > .inner, because it actually is the second element.

I therefore think, but I can't oversee the impact, to conditionally push currentSelector into parts. Something like this:

if (currentSelector !== '') {
  parts.push(currentSelector);
}
joshwaaaah commented 2 months ago

Hey,

First of all, great library, so thanks for all the work!

I've recently run into this issue with one of our packages utilising the :scope selector.

I've created a minimal example.

Is this something that may be fixed in future releases?

runarberg commented 2 months ago

Did :scope > selector stop working? I just upgraded from v12 to v14 and now all my tests that used :scope > selector fail

zthun commented 2 months ago

Did :scope > selector stop working? I just upgraded from v12 to v14 and now all my tests that used :scope > selector fail

I just ran into this as well using v14.11.0