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

Sort out the pre-compiler hints and filter list sources for Edge and Opera #2380

Closed
AdamWr opened this issue Jun 21, 2023 · 9 comments
Closed

Comments

@AdamWr
Copy link
Member

AdamWr commented Jun 21, 2023

AdGuard Extension version

4.2.133 beta

Browser version

Edge 114.0.1823.55

OS version

Windows 11

What filters do you have enabled?

AdGuard Base filter, AdGuard Tracking Protection filter, AdGuard URL Tracking filter, AdGuard Annoyances filter

What Stealth Mode options do you have enabled?

No response

Issue Details

Related to this issue - AdguardTeam/AdguardFilters#154327

Steps to reproduce:

  1. In Edge browser install AdGuard AdBlocker (Beta) 4.2.133
  2. Navigate to - https://forum.dobreprogramy.pl/
  3. Disable protection
  4. Enable protection
  5. Add this rule:
forum.dobreprogramy.pl##body
  1. Refresh website

Rule doesn't work.

Expected Behavior

Website should be blank.
Another problem is that disabling protection doesn't work correctly and ads are not displayed.

Screenshots

Screenshot 1:

image

Screenshot 2:

image

Additional Information

It seems that it's related to service worker, after unregistering it element hiding rule (forum.dobreprogramy.pl##body) works correctly.

@AdamWr AdamWr added the Bug label Jun 21, 2023
@ameshkov ameshkov changed the title Filtering does not work correctly on forum.dobreprogramy.pl Edge version 4.2.133 beta Sort out the pre-compiler hints and filter list sources for Edge and Opera Jun 21, 2023
@ameshkov
Copy link
Member

ameshkov commented Jun 21, 2023

After some internal discussions, here's my (hopefully) final suggestion:

  1. Keep using different lists for Chromium, Edge and Opera

  2. Change the logic for pre-compiler hints in the following way:

    • Chrome: adguard_ext_chromium
    • Opera: use two flags: adguard_ext_chromium + adguard_ext_opera
    • Edge: use two flags: adguard_ext_chromium + adguard_ext_edge
  3. Make the same change to the FiltersCompiler and make sure that edge/ and opera/ filter lists are built using this rule. Also, we should use the same exclusions in the platforms.json for all three.

  4. Update the knowledge base with the updated information: https://adguard.com/kb/general/ad-filtering/create-own-filters/#conditions-directive

@krystian3w
Copy link

krystian3w commented Jun 26, 2023

How will it work with uBo directives? Will it have to be encapsulated into two ifs independently? Example:

!#if env_chromium
!#if adguard_ext_opera
foo.bar##.opera
!
!
!#endif
!#endif
!#if env_chromium
!#if adguard_ext_edge
foo.bar##.edge-chromium
!
! maybe edge legacy too
!#endif
!#endif

or

!#if adguard_ext_chromium
!#if adguard_ext_opera
foo.bar##.opera
!
!
!#endif
!#endif
!#if adguard_ext_chromium
!#if adguard_ext_edge
foo.bar##.edge-chromium
!
! maybe edge legacy too
!#endif
!#endif

@ameshkov
Copy link
Member

Neither option will work.

Does uBO support conditions?

Something like that should be good:

!#if env_chromium || adguard_ext_opera
foo.bar##.opera
!#endif

@krystian3w
Copy link

It does not support and is not planned to be implemented e.g. due nesting is fixed in uBo 1.20.0+.
Then the filter "rule" for AdGuard is executed in uBo (condition marked as invalid is not ignored).

So it comes out that in the end it should be like this after all:

!#if !ext_ublock
!#if !ext_ubol
!#if env_chromium || adguard_ext_opera
foo.bar##.opera
!#endif
!#endif
!#endif

@ameshkov
Copy link
Member

I am not sure how to resolve this without conditions support.

Probably you'll need to use two different blocks for AG and uBO.

@ameshkov
Copy link
Member

It does not support and is not planned to be implemented e.g. due nesting is fixed in uBo 1.20.0+.

Nesting only solves && condition, it does not replace || which is needed more often to be honest.

Why was the feature request for conditions support rejected?

@krystian3w
Copy link

I would fudge that because of the small number of real-world usages/cases (high chance of parser corruption when the code is written only by gorhill and no one catches all the implementation bugs in the beta).

@gorhill
Copy link

gorhill commented Jul 15, 2023

Support for AdGuard-compatible proprocessing expressions was added a bit over two weeks ago: gorhill/uBlock@194354cd5d77. I was not aware of the issue here, just a coincidence that I decided to support this to increase compatibility.

@ameshkov
Copy link
Member

@gorhill looks great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants