AdguardTeam / AdguardBrowserExtension

AdGuard browser extension
https://adguard.com/
GNU General Public License v3.0
3.1k stars 331 forks source link

Complex/Long rules of native :is() :has() are not working in Adguard, works fine in other Adblockers. #2829

Open ghost opened 5 months ago

ghost commented 5 months ago

Please answer the following questions for yourself before submitting an issue

AdGuard Extension version

4.3.53 (beta) / 0.4.50 (Experimental MV3)

Browser version

1.68.61 Chromium: 126.0.6478.26 (Official Build) nightly (64-bit)

OS version

Windows 11 Version 23H2 (Build 22631.3672)

Ad Blocking

No response

Privacy

No response

Social

No response

Annoyances

No response

Security

No response

Other

No response

Language-specific

No response

What Stealth Mode options do you have enabled?

No response

Issue Details

Steps to reproduce:

  1. add twitch.tv##:is(.tw-tower > div, div:has(>[class*="search-result-card"]), .search-result-related-live-channels__row-container > :last-child > .tw-col):has([href])
  2. go to https://www.twitch.tv/directory/all
  3. All channels should be hidden but they are not (this is because no channel href is specified to hide channels base on the channel's name, so applies to every channel)
  4. Remove some parts of the rule like the :has() inside :is(), and it seems to work without issues, but that means the :has() rule has to be broken into an individual one for Adguard to work like other tested Adblockers.

Expected Behavior

No response

Actual Behavior

The rule is not being applied the browser gets weird like too slow because of the rule, if you remove the :is() and make them individual, the rules work and the browser feels fairly okay.

Screenshots

Screenshot 1

Additional Information

As you can see it is a very complex rule, but it is a completely fine and valid rule made with normal native Chromium features. There shouldn't be a reason why it is not working in Adguard.

And yes, I tested it and it works perfectly fine in other adblockers, Brave, uBlock and ABP which is MV3 complaint, so Devtools nor other adblockers have issues with it.

Why I started making these complex rules? because Brave doesn't support procedural filters at the moment, so I have to select channels to be hidden based on their attributes and since :has() is supported in Chromium browsers natively, it made it possible. I believe doing it with native selectors and not Procedurals is the way to go anyway, even if the rule is too complex, you don't get too much performance issues when hiding many channels compared to using the slower Procedural rules/non-native CSS selectors/pseudo-classes.

This rule is pretty much a native way of doing what 'unwanted twitch' extension does (an extension to 'block' channels based on tags, username and all that) but without the need of running the extension, especially on mobile where Chromium has no official extension support. I have been using it for a while, but seems to have issues only in Adguard when I wanted to see how my Brave custom rules worked in Adguard MV3, but noticed even the MV2 version has issues with it.

The reason for using :is() is because as you can see, without it, it rule has to be broken into 3 different rules, so it is better just to modify the href once and done.

But for example, the rule works by specifying a channel in the href attribute like :has([href="/smitegame"]) and it will hide individual channels if you go to https://www.twitch.tv/directory/category/smite from the directory and also the other selectors will hide the channel if you search for it https://www.twitch.tv/search?term=smitegame

So technically there is no reason for the rule to fail, when everything is valid in terms of Devtools and native browser CSS selectors/pseudo classes.

Like I said, this rule fails in both Adguard MV2 and MV3 and makes the browser perform really slow as well, so whatever it is trying to do, it is not doing it.

I tried to make the same rule in other sites that looked a little like this one, forcing some complexity like: :is(arg1 > .test, arg2, arg3:has(>.test2)):has([attr]) and they seemed to work, so I guess is because this rule has many > in the :is() part for each selector and also :has(>) and then another :has() on it. So I doubt many people will have issues with this, but it can happen and while breaking the rule to individual rules will work. I still wanted to report it, especially when it works fine in other adblockers so I thought just copy and pasting most rules like this would work in Adguard, but no.

Anyway, hope this has a fix so it's easier to use Adguard MV3 in other browsers that don't have native adblockers when Chromium moves to a MV3 only.

Thank you and have a good day.

slavaleleka commented 5 months ago

there are some known issues with :is() and :has() pseudo-classes in ExtendedCss: https://github.com/AdguardTeam/ExtendedCss#known-issues

ghost commented 5 months ago

I changed the name of this issue to reflect the fact that these are native selectors and not ExtendedCss ones. These are native and fully working/supported by Devtools and just as Adguard kb says, making the rules as ## should use native :is() and :has()

So unless Adguard thinks these are invalid for how many Child combinator exist inside :is() while having another selector with :has(), and then it expects you to add it as #?# but then it hits with limitations, then the issue shouldn't exist, especially when all other popular adblockers Brave (native), uBlock (MV2) and Adblockplus (MV3) the rule work.

I tried to make more rules like this and they also fail, because only 1 selector in :is() seems to be taken into account by Adguard (apparently the :has() one) and discards the other ones, so you have to make the rules with no Child combinator (or too many) if it has the :has() or keeping the child combinators and removing the :has() in :is(), so something is not computing correctly and as it should here.

BTW I tried to find simple blogs (don't mind them) that visualize things better since the Twitch rule applies to different parts of the website, and as you can see, the result is that only one of the :is() selectors is being taken into account, I assume that because only 1 element is being hidden by Adguard and ignoring the other elements. I added the Adguard logs where you can see and test the rules I used, and you can see what exactly Adguard says it is doing when applying these cosmetics.

Original Adguard Brave

Original Adguard Brave
ghost commented 5 months ago

BTW I saw another rule I made where :is() nested inside a :is() with :has() on it. It is still a valid native selector, it works in Devtools and Brave as well.

I made this testing one:


Filtering status: Modified Element: <div class="site-logo-desktop"> Type: HTML Source: cgchannel.com Rule: cgchannel.com##:is(.site-logo-desktop, .header-search, :is(.masterDescription, .container.homepage > div > *):has([href*="unreal"], [href*="unity"])) Filter: User rules

As you can see it only hid the logo, while it has to hide 8 elements.

If you remove the :has() and nest it like :is(:is()) (won't work the same but whatever) cgchannel.com##:is(.site-logo-desktop, .header-search, :is(.masterDescription, .container.homepage > div)) It will match the same 9 elements that Devtools reports is selecting with that. So the problem is not even nesting :is() inside :is() but if it has the :has() inside the :is().


Now, even nesting a :is() in :has() is working as intended by the native implementation. The rule: search.brave.com###results > .snippet:has(:is(.rich-snippet-content, [href*="speedtest"], [href*="fast"])) works, and while it is technically not needed and only adds complexity, it is allowed (doing :has(1, 2, 3) would be the same as :has(:is(1, 2, 3)) in terms of what an adblocker would do).

So the issue is not making weird nesting with what is allowed by the browser that might seem awkward, if the rule is valid then it should work and use the native implementation without issues. But the rule is hiding the 4 elements (in my case) that Devtools selects by visiting https://search.brave.com/search?q=test.

The only issue I can see is Adguard's log is only reporting 1 hidden element, <div id="definition" class="snippet standalone no-padding svelte-1agnhj"> but going through devtools I can see the display: none !important in all the 4 elements that it is suppose to hide, and I also see them show up when I turn Adguard off in the page.

Just making sure my report is covering every angle.

m-sarabi commented 2 months ago

I have the same problem. rule: w3schools.com###main > div:has(> a[href*="shop.w3schools.com"]) + hr , #main > div:has(> a[href*="shop.w3schools.com"]) It works correctly in the Element Blocking preview, but fails to apply after the page is reloaded:

If I break it into two rules, then only the div was blocked regardless of order:

w3schools.com###main > div:has(> a[href*="shop.w3schools.com"]) + hr
w3schools.com###main > div:has(> a[href*="shop.w3schools.com"])
slavaleleka commented 4 weeks ago

native :has() is not supported in the extension yet, but it is planned: https://github.com/AdguardTeam/AdguardBrowserExtension/issues/2587