Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Procedural filter causing full page block in Brave #43818

Open
1 of 6 tasks
Hryniuk1 opened this issue Feb 7, 2025 · 3 comments
Open
1 of 6 tasks

Procedural filter causing full page block in Brave #43818

Hryniuk1 opened this issue Feb 7, 2025 · 3 comments
Labels
OS/Android Fixes related to Android browser functionality OS/Desktop

Comments

@Hryniuk1
Copy link

Hryniuk1 commented Feb 7, 2025

Description

I am experiencing an issue with a procedural filter from the Adguard Annoyances filter list when using the Brave browser with the built-in adblock-rust. The filter in question is:
ya.ru,yandex.*##:matches-path(/images).ImagesViewer-Products
Filter list URL: https://filters.adtidy.org/extension/ublock/filters/14.txt
This filter is offered in uBlock Origin, although not enabled by default. It works correctly in uBlock Origin on both Chrome and Firefox, but in Brave, it results in the entire page being blocked, displaying only a white screen.
NOTE: There are similar procedural filters in the filter list like
ya.ru,yandex.*##:matches-path(/SOMETHING_ELSE)..., causing a white screen issue on other pages, such as yandex.com/SOMETHING_ELSE

Steps to reproduce

  1. Add the Adguard Annoyances filter list to Brave's Shield.
  2. Visit https://yandex.com/images
  3. Observe the white screen and full page block.

Actual result

The entire page is blocked, displaying only a white screen.

Expected result

The filter should block only the specified elements without causing the entire page to be blocked.

Reproduces how often

Easily reproduced

Desktop Brave version (brave://version info)

1.75.175 Chromium: 133.0.6943.54
Windows 11 Version 24H2 (Build 26100.2894)

Android device

  • Brand/model: Realme GT3
  • Android version: Android 15

Channel information

  • release (stable)
  • beta
  • nightly

Reproducibility

  • with Brave Shields disabled
  • with Brave Rewards disabled
  • in the latest version of Chrome

Miscellaneous information

No response

@Hryniuk1 Hryniuk1 added OS/Android Fixes related to Android browser functionality OS/Desktop labels Feb 7, 2025
@rebron
Copy link
Collaborator

rebron commented Feb 7, 2025

cc: @ryanbr

@ghost
Copy link

ghost commented Feb 7, 2025

I documented all these issues in #42873 and Brave team has not fixed it.

You can see in the comment #42873 (comment) how I mentioned exactly this, and how :matches-path besides the whole issue with it and how Procedurals depending on page doesn't apply properly, doesn't work if you don't place it at the end. uBlock allows both, at beginning or the end, but uBlock recommends it at the beginning to avoid issues, and that's what doesn't work in Brave.

You can read it in their wiki, the last paragraph https://github.com/gorhill/ublock/wiki/Procedural-cosmetic-filters#subjectmatches-patharg

Typically this procedural operator is used as first operator in a procedural cosmetic filter, so as to ensure that no further matching work is performed should there be no match against the current path of the current document location.

So, you need to create an exception ya.ru,yandex.*#@#:matches-path(/images).ImagesViewer-Products and move on, filter exceptions for cosmetics and scriptlets in the custom rules, custom lists and regional lists work, you just can't apply them to Default lists for now.
And That's all you can do, and then create your own filter if necessary.


I will mention this anyway, I think it is ridiculous to use Procedurals, just to use them, not even uBlock recommends the use of them, why? because they are not native selectors, they are made of JS, that means you could easily create Custom Scriptlets that work better than the current Procedurals and do the job, but it will always be slower than native CSS selectors that only modify CSS properties.
That means, is there any reason for :matches-path to hide images like this? probably not, but anyway, some people just use dumb approaches and complicate things unnecessarily.
It's like yesterday, Brave released Custom Scriptlets for Brave stable, they made a blog post and one of their examples is using it to .remove() some elements and apply with 100% to another element... please don't do that, use normal cosmetics and CSS native selectors and :style() action operator to do that, and then I saw in the Twitter/X comments someone doing the same... like CSS selectors are the fastest zero impact in performance way of doing things, yet, people still use Procedurals for laziness and now JS for doing what Brave has been able to do for years, especially since Chromium started to support :has() pseudo-class, being able to easily do crazy filters that will be faster than :matches-path :has-text and any of those things.

As uBlock says in their wiki :

Only use procedural cosmetic filters when plain CSS selectors won't work.

and this one to explain what I already said:

Procedural means JavaScript code will find DOM elements that it must hide. A procedural cosmetic filter uses a filter operator that will tell uBO how to find/filter DOM elements to find which DOM elements to target.

Anyway, the issue is Brave hasn't fixed their Procedural Filtering knowing it is currently buggy... so it's better to avoid it at all cost, even more than in uBlock where it works fine (but slow).

I reported it 2 months ago, with no fix yet, so even if you change the :matches-path from position, it might not even do what it is suppose to do, but since Procedurals are like the worst thing to use and you can create Custom Scriptlets, I guess, you can easily create your own "Procedurals". So it is not the end of the world if native CSS selectors are not enough for some websites.


PS. This list is also available in Brave, it's the regional list in brave://settings/shields/filters called Adguard Russian, but again, the exception will work fine since it is not a Default list, so no need to add any list to test this, just enable the regional list.

PSS. Also, Cosmetics don't "Block" they hide... all cosmetics do (Native or Procedural) is to inject the CSS declaration display: none !important;. Saying Block is wrong and it will always be wrong because there is nothing being blocked, the only one that blocks is Network Request Filtering, and in some cases Scriptlet Injection, because JavaScript has the power to block some APIs from happening and all that.

@Hryniuk1
Copy link
Author

@ghost
I don't understand the details of how filters work, so I can hardly support the discussion.
Adguard Russian is, as far as I understand, a filter for russian sites ( https://filters.adtidy.org/extension/ublock/filters/1.txt ), while Adguard Annoyances is another
Special thanks for the hint with the exception, it helped both to save the filter and to "cure" the site.

@Hryniuk1 Hryniuk1 reopened this Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS/Android Fixes related to Android browser functionality OS/Desktop
Projects
None yet
Development

No branches or pull requests

2 participants