:scope selector direct children query is not working because of tag name resolution strategy

[sasensi]

This was originally reported as part of the jsdom repo here.
I decided to try to fix it (because I had the exact same issue) and was able to track down the issue until here.

So here's the case: given the following DOM tree:

<div>
  <div class="ok"></div>
  <div class="ok">
    <div class="ko"></div>
  </div>
</div>

Getting a reference to the root <div> and doing the following query:

rootElement.querySelectorAll(':scope > div')

Doesn't only return the 2 expected "ok" divs but also the "ko" one.

Here's the full reproduction code allowing to easily switch between an ok and a ko case:

NW.Dom.install();

const SHOW_BUG = true;

const rootElementTagName = SHOW_BUG ? 'div' : 'main';

const html = `<${rootElementTagName}>
  <div class="ok"></div>
  <div class="ok">
    <div class="ko"></div>
  </div>
</${rootElementTagName}>`;

const dom = new window.DOMParser().parseFromString(html,'text/html');
const rootElement = dom.querySelector(rootElementTagName);
const queryResult = rootElement.querySelectorAll(':scope > div');

for (const element of queryResult) {
  console.log(element.className);
}

// This should always output:
// - ok
// - ok
//
// But the actual output is:
// - ok
// - ok
// - ko

And here's a fiddle hosting it.

---

I don't know this library codebase well, but I investigated a bit and I have the feeling that this is caused by the fact that this pattern : :scope > div is turned into a more generic one : div > div which is solved using a parent tag name resolution strategy and ultimately returns all the element globally matching this pattern: div > div, somewhat ignoring the element context.
I hope that this can help saving time to fix it.

[dperini]

@sasensi
I am not 100% sure the following is the correct way of definitely solving the ":scope" pseudo-class.
However in the "test/scope" directory there are a few test passing and I would add your test to the existing list.
So please try to substitute the 7 lines long "makeref" method found on lines 1330-1336 with the following modified code:

  makeref =
    function(selectors, element) {
      return selectors.replace(/:scope/ig,
        element.localName + 
        (element.id ? '#' + element.id : ':not([id])') +
        (element.className ? '.' + element.classList[0] : ':not([class])'));
    },

Thank you in advance for your contribution and for reporting the bug.
Let me know if this change completely solves your current problem.

[sasensi]

Hey @dperini, thank you for you solution proposal, I'll try it asap and get back to you.

[sasensi]

@dperini, I looked at your solution and I also looked more closely at the codebase.
I now understand that the handling of :scope is quite basic for now, and rely on the fact that the root element must be identifiable via a global selector (e.g. an id attribute).
I think that a more robust and flexible solution would be to implement a way to filter out all elements that are not direct children of the root element by using the element.parentElement property.

As an example here's a simple way of solving this specific case outside of the library:

const rootElement = dom.querySelector(rootElementTagName);
const queryResult = rootElement.querySelectorAll(':scope > div').filter((it) => it.parentElement === rootElement);

Do you think that this kind of strategy could be included into the library ?
(I saw that you already use internal callbacks to do checks in the compiled code, so maybe the same mechanism could work for this case ?)

[dperini]

@sasensi
there were no advances that I know on browsers consensum about the functionality of the :scope pseudo class.
I cannot consider this a bug nor an issue. Feel free to reopen in case you have reproducible results on all browsers.

[Perfectoff]

@sasensi there were no advances that I know on browsers consensum about the functionality of the :scope pseudo class. I cannot consider this a bug nor an issue. Feel free to reopen in case you have reproducible results on all browsers.

Hi, Diego. Here is a link to the description of the :scope pseudo-class in the W3C standard used by most browsers.
It says:

For example, if you have a DocumentFragment df, then df.querySelectorAll(":scope > .foo") matches all the .foo elements that are "top-level" in the document fragment (those that have the document fragment as their parentNode).

So your solution, given above and implemented in the library, is wrong. It gives the correct result only if the element has an id, otherwise it refers to all similar elements (that have the same tag and class), which is incorrect.

[dperini]

@Perfectoff @sasensi
it sounds like it should be easy enough.
If I understand correctly the new specifications define this special pseudo-class as the top-level children having the scoped element as their parentElement. I will have a try at it asap since it seems many developer use this.
Thank you to bring this to my attention again.