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

Add compatibility with some of ABP's script filters. #1011

Closed
4 of 8 tasks
kulfoon opened this issue May 5, 2020 · 13 comments
Closed
4 of 8 tasks

Add compatibility with some of ABP's script filters. #1011

kulfoon opened this issue May 5, 2020 · 13 comments
Labels
declined declined enhancement New feature or request

Comments

@kulfoon
Copy link

kulfoon commented May 5, 2020

Prerequisites

  • I verified that this is not a filter issue
  • This is not a support issue or a question
  • I performed a cursory search of the issue tracker to avoid opening a duplicate issue
    • Your issue may already be reported.
  • I tried to reproduce the issue when...
    • uBlock Origin is the only extension
    • uBlock Origin with default lists/settings
    • using a new, unmodified browser profile
  • I am running the latest version of uBlock Origin
  • I checked the documentation to understand that the issue I report is not a normal behavior

Description

An idea/enhancement to further discuss - what do you think.

uBO currenlty supports / is compatible with some ABP's and AG's script filters for. ex.:

and with some more as well.

Some time ago ABP implemented Snippet filters / Snippet Filters Tutorial .
Some of them - the most common ones, are compatible with uBO, they even wrote that the idea originates from uBlock Origin, for. ex.:

website.com#$#abort-on-property-read property could be read as/passed to
website.com##+js(abort-on-property-read.js, property)

website.com#$#abort-on-property-write property could be read as/passed to
website.com##+js(abort-on-property-write.js, property)

website.com#$#abort-current-inline-script property string could be read as/passed to
website.com##+js(abort-current-inline-script.js, property, string)

website.com#$#uabinject-defuser could be read as/passed to
website.com##+js(uabinject-defuser.js)

I know they are limited to Custom Filters and ABP Anti-Circumvention Filter List, but perhaps ABP Anti-Circumvention Filter List could be included and enabled in uBO by default (it's included and enabled by default in ABP as well), what would reduce a bit number of issues reported to uAssets tracker / filters being added to uBO, what would relieve a bit uBO filter creators. Still, issues not covered by ABP Anti-Circumvention Filter List could be fixed in uBO.

Your environment

  • uBlock Origin version: 1.26.3b16
  • Browser Name and version: Firefox 75 64bit / Opera 68 64bit
  • Operating System and version: Windows 7 SP1 64bit
@gorhill
Copy link
Member

gorhill commented May 5, 2020

This was discussed internally a long time ago and declined. There is no list used by uBO which make use of this syntax, it's currently only used by ABP's own "Anti-CV" list, which is not to be used in uBO because it would likely break uBO's own anti-blocker solutions.

@gorhill gorhill closed this as completed May 5, 2020
@gorhill gorhill added the declined declined label May 5, 2020
@uBlock-user uBlock-user added the enhancement New feature or request label May 5, 2020
@kulfoon
Copy link
Author

kulfoon commented May 5, 2020

Oh shit, I apologize then for opening an issue, somehow missed the comment you reffered, yeah, well, shit happens.

@peace2000
Copy link
Member

peace2000 commented Jul 21, 2021

@gorhill some time ago @DandelionSprout made a request https://gitlab.com/eyeo/adblockplus/adblockpluscore/-/issues/229 for ABP developers that they would add support for uBO's :has and :has-text and convert them automatically to their ABP counterparts :-abp-has and :-abp-contains. They were about to actually implement that, but now they started to worry about upcoming CSS4 standard and it's native :has selector if that would cause some issues as uBO uses the same syntax currently for a procedural filter. See discussion at merge request here: https://gitlab.com/eyeo/adblockplus/adblockpluscore/-/merge_requests/513

Can you give some comment to them on Gitlab? (You can login to Gitlab via a Github account).

I know it's not your pain what ABP does but as a filterlist maintainer I really hope that there were better compatibility between different adblockers. But as I'm not uBO's developer I cannot make exact comments about :has selector and it's future.

@gorhill
Copy link
Member

gorhill commented Jul 21, 2021

they started to worry about upcoming CSS4 standard and it's native :has selector if that would cause some issues as Ubo uses the same syntax currently for a procedural filter

uBO detects at filter compile time whether a filter is to be used natively or transformed into an emulated CSS selector, for instance, this is going to compile into a native CSS selector-based filter:

##div:not(.class)

While this is going to compile into an emulated CSS selector:

##div:not(.class > .other)

Same will apply for :has() if and when it becomes natively supported by browsers, with no change required in uBO's code.

I have always been open to support as many sources of filter lists as possible, hence why uBO is compatible with AdGuard syntax where it makes sense, and also with ABP-specific syntax where it makes sense, while ABP has steadfastly ignored the work already established by both AdGuard and uBO to go its own way with their own syntax.

I am not going to invest my time arguing with them to be a better blocker and be more compatible with quality filter lists from other sources, they should want this on their own.

@gorhill
Copy link
Member

gorhill commented Jul 21, 2021

By the way, related to the original issue here (ABP's "snippet filters"), ABP is moving some of its development out of public eye, see https://gitlab.com/eyeo/adblockplus/abp-snippets (ref). That kind of move makes it clear to me they are not interested in sharing, collaborating and being really community-driven -- they will be free to look at uBO/AdGuard work in that field which occurs in full public view, while shielding their own snippet code work behind a private repo.

@WebReflection
Copy link

no change required in uBO's code

The MDN page you pointed at is not fully aligned with what is landing.

We actively collaborated with Igalia and they did a huge job to go ahead and propose a :has() selector syntax that doesn't backfire, and there are possible gotchas specifically with the :scope part of the inner selector.

This is the reason we raised a concern, whenever current :-abp-has(), or possibly uBO :has(), could be future hostile, as I believe it'd be impossible to distinguish between two :has() selectors and figure it out which one was the uBO one, which one is the actual shipping standard, and which one is the filter's author's intent.

ABP is moving some of its development out of public eye

This is a bit speculative, but it's also not the venue, for me, to provide extra details on that move, or what's next, and it'd be great if we could keep the conversation relevant to what @peace2000 asked, as all we are trying to do, maybe late, but I've personally never been involved in those old decisions, is to simplify the effort for filters' authors, and because we know :has() might be slightly different, we'd like to be careful, and be aligned, with others, to simplify their life, and never have ambiguous syntax.

Thanks in advance for your collaboration.

@gorhill
Copy link
Member

gorhill commented Jul 22, 2021

I tried the selectors listed in that commit, and found that uBO's :has() fails with the forms:

.target:has(:scope + ...)
.target:has(:scope ~ ...)

However, it succeeds when removing leading :scope . With a small local fix, I could make those cases work when :scope is explicitly provided.

When I look at the use of :has() in uBO filters (156 instances) and AdGuard Base (943 instances), I can't find cases which are outside the spec, most of what I see is plainly as per spec -- i.e. :has(> div.sidebar-promo), :has([id^="div-gpt-ad"]), etc.

Except of course for when :has() contains other procedural operators (i.e. :has(.foo:has-text(...))), in which case these selectors would be found to be invalid at compile time and be handled has procedural cosmetic filters instead of native CSS selector-based cosmetic filters, just as it is now.

I can't imagine a scenario where :has()'s semantic would become so different from the current one such that this would cause an issue.

I don't know how @ameshkov feels about this.

@WebReflection
Copy link

WebReflection commented Jul 22, 2021

In ABP we are transforming :has(> div.sidebar-promo) into :has(:scope > div.sidebar-promo) and we need to somehow adjust that transformation because it might cause issues, but as the final standard is being shaped as we speak, we'd like to avoid blindly converting ##:has(...) into #?#:-abp-has(...) because intents vs semantics might be slightly different.

We understand such difference shouldn't be huge, and we probably need to counter parse nested :has(:has()) too, although we explicitly have this separation between ## and #?# where custom procedural cosmetic filters rely on our own implementation, while ##:has(...) could be already used as native CSS4 implementation.

If we decide to go for #?#:has() to #?#:-abp-has() transformation though, we're not sure if that solves anything, as the filter is still considered emulation, requiring the different definition, hence my question: does uBO understand #?#:has(...) if found in the wild, normalizing it as ##:has() which seems to be the preferred uBO way?

Thanks.

@gorhill
Copy link
Member

gorhill commented Jul 22, 2021

does uBO understand #?#:has(...)

In uBO ? in #?# is currently silently dropped internally such that the filter becomes equivalent to the form ##.

Forcing a filter to be parsed as extended CSS filter when there is a ? is something I can consider supporting, as though AdGuard does not require this, it strongly suggests to use ? for extended CSS.

@WebReflection
Copy link

In uBO ? in #?# is currently silently dropped internally such that the filter becomes equivalent to the form ##.

That's pretty awesome, because then we can keep our own separation and just consider :has filters like :-abp-has in our elemhide emulation logic, preserving, for the time being, ##:has for future improvements, once the standard is out.

Thanks for these details, it helped moving forward with our issue.

gorhill added a commit to gorhill/uBlock that referenced this issue Jul 23, 2021
@Yuki2718
Copy link

Yuki2718 commented Aug 13, 2021

Regarding :scope something strange happened on https://altema.jp/. I added altema.jp##.slick-track > li:not(:has(> div)) to AGJPN, but happened to have found altema.jp##.slick-track > li:not(> div) works just the same way with 3 matches on the page tho I don't know what this filter means. The logger reports the filter as altema.jp##.slick-track > li:not(:scope > div), however, with :scope added this filter has 13 matches, hiding all li.

@gorhill
Copy link
Member

gorhill commented Aug 13, 2021

with 3 matches on the page

Which page? I see zero match on the front page.

however, with :scope added this filter has 13 matches

Unexpected.

If you try the following in the dev console:

document.querySelectorAll('.slick-track > li:not(:scope > div)');

It works, this means the selector is deemed valid by the browser -- so it does not go through uBO's pseudo cosmetic filtering.

However it unexpectedly returns a collection of li elements with each having a direct div child, not something I would expect from such selector, since I read it as returns a collection of li elements which have no direct child div element.

@gorhill
Copy link
Member

gorhill commented Aug 13, 2021

The logger reports the filter as altema.jp##.slick-track > li:not(:scope > div)

That's wrong, the fix I implemented is supposed to normalize without :scope, that was the intent. However as said I can't reproduce a case matching that filter, I need a way to reproduce on my side so that I can investigate why the logger is reporting the filter with :scope in it.

gorhill added a commit to gorhill/uBlock that referenced this issue Oct 27, 2021
Related issue:
- uBlockOrigin/uBlock-issues#1751

Related feedback:
- https://www.reddit.com/r/uBlockOrigin/comments/qgd6fe/

It turns out Chromium has started to implement the `:has()`
operator, which becomes recognized when the browser flag
"Experimental Web Platform features" is enabled. However the
hic is that `:has()` is not supported as a declarative CSS
style rule and is only supported through `querySelector()`
et al.

The fix is to no longer detect plain CSS selectors through
`querySelector` et al. but rather use an actual stylesheet
to validate that a cosmetic filter can be injected into a
stylesheet in a declarative way.

Additionally, I added support to enforce ABP's semantic
regarding cosmetic filter with the `#?#` anchor: when using
such anchor, uBO will _first_ try to compile the filter as
a procedural one rather than a declarative one.

Related discussion:
- uBlockOrigin/uBlock-issues#1011 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
declined declined enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants