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

:has(+ selector) regression. #167

Open XX-J opened 1 year ago

XX-J commented 1 year ago

В расширении v4.1.39 на FireFox v102.6 ESR следующие два примера работают, в более новых версиях расширения - нет.

1. Скрытие маленького рекламного баннера над списком писем в почте Mail.ru правилом e.mail.ru##div:has(+ .only-new-toolbar). Обязательно: открывать какую-нибудь папку, а не письмо, например https://e.mail.ru/inbox/.

Screenshot: ![2023 04 15-1](https://user-images.githubusercontent.com/17948007/232245271-0f951b4f-864a-4165-b901-d5f5d392a55a.png)

2. Скрытие главного, большого баннера на главной странице https://www.rambler.ru/ правилом www.rambler.ru##div:has(+ .commercial-branding). Причём правило www.rambler.ru###app > div:has(+ .commercial-branding) работает и в v4.1.53 и в v4.1.55

Screenshot: ![2023 04 15-2](https://user-images.githubusercontent.com/17948007/232245385-26dcdc63-b9f0-4d88-8d08-6c7e909d1633.png)
ameshkov commented 1 year ago

Есть приличный шанс что :has мы теперь нативно применяем, его же в браузеры добавили.

Попробуйте document.querySelectorAll протестировать в браузерной консоли, отличается ли результат?

XX-J commented 1 year ago

Есть приличный шанс что :has мы теперь нативно применяем, его же в браузеры добавили.

Нет, и, надеюсь, в ближайшее время вы не будете его нативно применять, т.к. в Chromium его добавили только со 105-й версии, а в FireFox со 103-й, и то, только через принудительное включение. + Ваш :has круче нативного - он может быть вложен в другой :has (по крайней мере так в вашей справке).

И, я же писал:

В расширении v4.1.39 на FireFox v102.6 ESR следующие два примера работают... Причём правило www.rambler.ru###app > div:has(+ .commercial-branding) работает и в v4.1.53 и в v4.1.55

krystian3w commented 1 year ago

Nesting has usually doesn't make sense, you can rewrite 99% of the rules into a single using has that checks the whole string path at once. Nesting is unlikely to slow down rule so much that a flawed script could not generate an "ad blocking detected" message because of it.

Such a thing is not nesting:

Detecting that you prefer to use ExtendedCSS (nesting something not allowed in CSS like contains, matches-css), should be fixed mostly within the ticket: https://github.com/AdguardTeam/ExtendedCss/issues/149.


в более старших версиях расширения - нет

understood as: in older versions of the extension - no

Besides, rather 99 percent of add-ons don't support reverse repair - I wouldn't expect the release of 4.1.37.1 / 4.1.36.1 / 4.1.22.1 ... 4.0.204.1 (or any old one, e.g., for Firefox 52-56 on line 3.6.17.1) as a fix.

As for repairing it to work without "#app >", I see no contraindication (as long it is not the new mechanism limiting JS checking of several hundred divs on a page - 7 checks vs 599 checks), but in release 4.1.56+.

XX-J commented 1 year ago

Detecting that you prefer to use ExtendedCSS (nesting something not allowed in CSS like contains, matches-css)...

No. I mean only: e.mail.ru##div:has(+ .only-new-toolbar) and www.rambler.ru##div:has(+ .commercial-branding).

в более старших версиях расширения - нет understood as: in older versions of the extension - no

Is not right. Understood as: "...in upward versions of the extension - no." I.e. in v4.1.46 and up - no.

I see no contraindication (as long it is not the new mechanism limiting JS checking of several hundred divs...

Both rules should to work, with #app > or without.

Speaked "...new mechanism limiting JS checking of several hundred divs..." you mean limitation of "Manifest v3" or ..?

krystian3w commented 1 year ago

I am more concerned with assessing whether the rule is efficient regardless of the manifest.

Perhaps based on Mozilla's telemetry, it was determined how often the message "AdGuard slows down the browser/page" is seen and linked to rules that the user manually adds, e.g. checking 500 elements with ExtendedCSS.

XX-J commented 1 year ago

Mozilla's telemetry is off. I don't seen message "AdGuard slows down the browser/page". 599 checks divs is not a lot (IMHO).

slavaleleka commented 1 year ago

it's just a bug of ExtendedCss v2, it shall be fixed in future

we do not apply the :has() pseudo-class natively yet. and Firefox does not support it yet too