search

AdguardTeam / ExtendedCss

A TypeScript library for non-standard element selecting — :contains(), :matches-css(), etc., and applying CSS styles with extended properties.
GNU General Public License v3.0
61 stars 9 forks source link

Combining selectors with commas, followed by :has(-text), gives very dramatically different results between AdGuard and uBO #118

Closed DandelionSprout closed 3 years ago

DandelionSprout commented 3 years ago

I've found what may or may not be a bug, reproduced in both AdGuard for Windows and Android.

With the custom example entry nrk.no##span,section,kur-room__title:has-text(korona) on www.nrk.no:

uBlock Origin: Blocks span, section and/or kur-room__title elements containing the text korona AdGuard: Blocks all span and section elements, as well as kur-room__title elements containing the text korona

I vastly prefer the former approach and have utilised it for e.g. Anti-FiM List, which works amazingly for uBO users, but which would cause AdGuard users to see near-empty pages on sites like YouTube, Deezer, DailyMotion and Mastodon.

Therefore I want to ask whether the latter approach is something that you guys chose to do intentionally, or if it's something that needs to be fixed?

ameshkov commented 3 years ago

Well, the way it works in uBO is wrong according to the CSS spec and I'd prefer not to repeat this. This may cause problems in the future. Imagine that at some point :has finally becomes a part of CSS and the rules that were previously applied using javascript, will use standard CSS instead. When this happens, all those rules that were relying on this feature will completely unexpectedly change the way they work.

On a side note, I don't like combining multiple CSS selectors in one rule. It makes it harder to analyze rules and figure out which one is not necessary anymore.

CC @gorhill is this a bug or a feature?

gorhill commented 3 years ago

By design -- the advice is to not use commas this way when using procedural operators.

Supporting commas exactly as in CSS would require a full CSS parser, which is a lot of code and burdensome for content scripts. I believe you just extend Sizzle, so this come free for you but on my side I rather keep the content scripts as lean as can be at the expense of having to split a cosmetic filter in two, which is trivial for filter authors.

Documentation:

Important: ... Concatenating multiple procedural selectors in one filter is not supported. example.com##p:has(img),div:has-text(advert) will not work as expected (#453).

gorhill commented 3 years ago

Looks like :is is now supported?

If so, something like this should work in both AdGuard and uBO with same result:

##:is(span,section,kur-room__title):has-text(korona)
ameshkov commented 3 years ago

Looks like :is is now supported?

Does not seem to work with document.querySelectorAll.

Anyways, @DandelionSprout, we'll implement :is as a part of ExtCSS and this would allow using rules like this.

DandelionSprout commented 3 years ago

https://developer.mozilla.org/en-US/docs/Web/CSS/:is claims that :is is Firefox-only, for the record.

gorhill commented 3 years ago

Does not seem to work with document.querySelectorAll

I tried in Firefox, it worked. It seems Chromium does not support it yet, but rather support the alias :-webkit-any.

ameshkov commented 3 years ago

Works in Chrome Canary so it's not too long till it starts working everywhere.