The fix for elem > :operator seems to be not right

[jspenguin2017]

Describe the issue

The fix here: 37fde84#diff-1c951eedcd0be2e11c02da8fabcc46b5R336
seems to be not right, as the 3 cases have different behavior.
Simply adding * will cause the selector to behave differently.

One or more specific URLs where the issue occurs

http://example.com/

Screenshot in which the issue can be seen

Steps for anyone to reproduce the issue

1. Enter test link
2. Execute the 3 selectors
3. Observe different result

Your settings

All default

• OS/version: Win10
• Browser/version: Chrome
• uBlock Origin version: N/A

Your filter lists

All default

Your custom filters (if any)

Nothing

[gorhill]

Transcribed from pic for convenience:

document.querySelectorAll('body > div > :not(p)');
document.querySelectorAll('body > div :not(p)');
document.querySelectorAll('body > div > * :not(p)');

I don't get the issue: these cosmetic filters are not procedural filters, while the commit you point at is strictly for procedural filters.

So this issue currently is just speculated, there is no real repro steps to make the case appending * is wrong. But let's assume :not is procedural: I don't see any discrepancy in results when appending * to a dangling selector:

Same results:

• NodeList [h1]
  • document.querySelectorAll('body > div > :not(p)');
  • document.querySelectorAll('body > div > *:not(p)');
• NodeList(2) [h1, a]
  • document.querySelectorAll('body > div :not(p)');
  • document.querySelectorAll('body > div *:not(p)');
• NodeList [a]
  • document.querySelectorAll('body > div > * :not(p)');
  • document.querySelectorAll('body > div > * *:not(p)');

[jspenguin2017]

As seen in the image of that post:
https://user-images.githubusercontent.com/7283682/34429143-5c4ecc10-ec11-11e7-94a3-413008b0ff36.png

If you simply add *, it will be duplicate of the rule below, however, ameshkov stated that it is not a duplicate: NanoAdblocker#1 (comment)

I also tested with Adguard extension, the two rules indeed behave differently.

[gorhill]

the two rules indeed behave differently

What "two rules"? Can you please transcribe these rules here? I posted pairs of rules above to show that appending * to make explicit what is implicit does not change the behavior of the selector. You can conveniently copy/paste these code snippets to your console and see the result.

[gorhill]

Looking at the pic, you were actually right that the two rules could become one:

dobreprogramy.pl##body > .page_form :matches-css(...)

Replaces both:

dobreprogramy.pl##body > .page_form > :matches-css(...)
dobreprogramy.pl##body > .page_form > * :matches-css(...)

Yes, the rules were not the same, both they could be combined into one, so they were redundant in a way.

[jspenguin2017]

Oh, true, I missed that part, however, dobreprogramy.pl##body > .page_form > :matches-css(...) is not a duplicate of dobreprogramy.pl##body > .page_form > * :matches-css(...) because one matches only immediate children of .page_form and other matches only children of immediate children of .page_form.

Sure they can be combined into one filter that matches all children of .page_form, but that's only this specific case. What if you only have one of the two filters? Then it'll be different.

[gorhill]

because one matches only immediate children of .page_form and other matches only children of immediate children of .page_form`

Sure, I understood that part, I never argued against this. I am merely still trying to understand why you think the fix in the commit is wrong -- and somehow you brought the case above as being relevant to the issue reported here.

Why do you believe:

document.querySelectorAll('body > div > :not(p)');

Is not the same as:

document.querySelectorAll('body > div > *:not(p)');

The second form is what the fix does: add an explicit * when the selector is "dangling".

[jspenguin2017]

I believe it's different because Chrome spit out different node list, and you said:

Pseudo operators are meant to mimick what could be a valid CSS selector

[gorhill]

it's different because Chrome spit out different node list

Please be loquace, it's starting to feel like pulling teeth. What exactly are the two selectors causing two different node lists?

[jspenguin2017]

OK fine, I'll fix it myself.

[gorhill]

You are asking me to read your mind.

Look at what I had to go through in order to have a case for me to try, to no avail.

I took the time to provide you cases demonstrating the opposite of what you state (and courteously providing you all this in text which you can easily copy/paste).

For some reasons you have been unwilling to show make the exact issue, including closing the issue here with no further constructive comment, as if me asking you only the two single selectors with which I can reproduce the purported issue unambiguously was asking too much.

[jspenguin2017]

It's really simple:

• body > div > :not(p) is not equivalent to body > div > * :not(p)
• dobreprogramy.pl##body > .page_form > :matches-css(...) is not equivalent to dobreprogramy.pl##body > .page_form > * :matches-css(...)
• The new patch treat dobreprogramy.pl##body > .page_form > :matches-css(...) to be the same as dobreprogramy.pl##body > .page_form > * :matches-css(...)

I have all explained in posts before, I don't know what is not clear.

[gorhill]

body > div > :not(p) is not equivalent to body > div > * :not(p)

Of course it's not -- I never ever thought it was. The fix does not cause such discrepancy. The fix just adds a * to a dangling selector. With the fix, the following selector:

body > div > :not(p)

Becomes:

body > div > *:not(p)

Not:

body > div > * :not(p)

A single * is appended, not a * followed by a space.

[jspenguin2017]

Well, either way something isn't right with your fix, if I understood right, you said with your fix,

example.com##div > :has(a)

is translated to

example.com##div > *:has(a)

But:

[gorhill]

I can reproduce. However the element isn't removed because an exception is thrown. For some reasons the prefix selector is an object rather than a string.

[jspenguin2017]

Eh, I guess I read the code wrong...
Well, I only got a theoretical case and not a real one, you said last time you don't want theoretical issues so I didn't post it in the opening.
Should've check the console... My bad.

[gorhill]

Ok, it's because I modified :has() to behave like :if(), because uBO was rejecting many filters from Adguard lists which were using :has with procedural selectors as args, while uBO wouldn't allow this.

[gorhill]

I really need a test page that would at least test most filters at once and this would allow me to quickly assess whether something is broken. The breakage was bad here, essentially all :has filters were broken, but I ended up focusing strictly on the parsing, and I did not pay attention to the end result.