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

Restore :if-not() support as an alias of :not(:has()) #2376

Closed
8 tasks done
Yuki2718 opened this issue Nov 21, 2022 · 9 comments
Closed
8 tasks done

Restore :if-not() support as an alias of :not(:has()) #2376

Yuki2718 opened this issue Nov 21, 2022 · 9 comments

Comments

@Yuki2718
Copy link

Yuki2718 commented Nov 21, 2022

Prerequisites

  • I verified that this is not a filter list issue. Report any issues with filter lists or broken website functionality in the uAssets issue tracker.
  • This is not a support issue or a question. For any support, questions or help, visit /r/uBlockOrigin.
  • I performed a cursory search of the issue tracker to avoid opening a duplicate issue.
  • The issue is not present after disabling uBO in the browser.
  • I checked the documentation to understand that the issue I am reporting is not normal behavior.

I tried to reproduce the issue when...

  • uBO is the only extension.
  • uBO uses default lists and settings.
  • using a new, unmodified browser profile.

Description

No support of :if-not, though rarely used, can cause problems.
AdguardTeam/AdguardFilters#135390

A specific URL where the issue occurs.

https://www.colordic.org/gradation

Steps to Reproduce

  1. Visit the site with Japanese filters enabled and Unbreak disabled, you'll see nearly all contents hidden due to colordic.org#?#footer > div.container > div.row:if-not(.text-muted).

Expected behavior

:if-not() should be converted to :not(:has())

Actual behavior

:if-not() is ignored, causing false positive.

uBO version

1.45.2

Browser name and version

Chrome 107.0.5304.107

Operating System and version

Windows 10

@Yuki2718
Copy link
Author

Yuki2718 commented Nov 21, 2022

Will add temp fix to Unbreak. According to AG document,

This pseudo-class is basically a shortcut for :not(:has()). It is supported by ExtendedCss for better compatibility with some filters subscriptions, but it is not recommended to use it in AdGuard filters. The rationale is that one day browsers will add :has native support, but it will never happen to this pseudo-class.

So I guess if-not has been used by some minor lists too.

Yuki2718 added a commit to uBlockOrigin/uAssets that referenced this issue Nov 21, 2022
@Yuki2718
Copy link
Author

Oh, forgot uBO once supported if-not.

@Yuki2718 Yuki2718 changed the title Support :if-not() as an alias of :not(:has()) Restore :if-not() support as an alias of :not(:has()) Nov 21, 2022
@gorhill
Copy link
Member

gorhill commented Nov 21, 2022

:if-not() is not in uBO's own documentation, and AdGuard discourages its use, so the AdGuard filter list person surely should not be using it? I don't want to keep piling on deprecated code just for the sake of someone somewhere might use it. The AdGuard person who used :if-not(...) should be told to use :not(:has(...))) instead, that would easily solve the issue for all.

@gorhill
Copy link
Member

gorhill commented Nov 21, 2022

I am thinking of removing AdGuard Base from uBO's stock lists. The main reason it was added a long time ago was because unlike EasyList, it was using special style(...) syntax which was beneficial to uBO, at a time when uBO did not have its own solid well maintained lists. This is no longer the case, and seeing that now exception filters are added in Unbreak just for the sake of excepting a filter which is not part of default lists, it's probably time to drop AdGuard Base to reduce the burden of having to mind incompatibility.

@Yuki2718
Copy link
Author

My exception is for Japanese filters, a default regional list.

@gorhill
Copy link
Member

gorhill commented Nov 21, 2022

Oops my bad. So the question to AdGuard filter list committers: why use :if-not(...) when its use is not recommended in AdGuard's own documentation? Large code base such as uBO or AdGuard have to be willing to deprecate bad ideas from the past. I kept :if-not(...) for a very long time after it was marked as deprecated, so at some point I have to let go of those deprecated code paths for the sake of keeping the code base manageable. Having to constantly mind that nothing break is a burden, and having to do so also for deprecated code paths is an added burden I can do without. Compare this with the triviality of using :not(:has(...)) instead of :if-not(...) by a filter list maintainer.

@Yuki2718
Copy link
Author

Yuki2718 commented Nov 21, 2022

I contacted @zloyden but he's currently away. I'll change the rule and remove from Unbreak.
AdguardTeam/AdguardFilters@f7e33ff
AdguardTeam/AdguardFilters@8e6016a
uBlockOrigin/uAssets@aa12288

@gorhill
Copy link
Member

gorhill commented Nov 21, 2022

By the way, div.row:if-not(.text-muted) can't be enforced declaratively by the browser, while div.row:not(:has(.text-muted)) is enforced declaratively by the browser (at least in uBO) -- i.e. it's part of the injected stylsheet and the browser does the filtering.

@krystian3w
Copy link

krystian3w commented Nov 29, 2022

Duplicate of #2228 (comment)


Also removed from support is :if(...) / [-ext-has]:

! all "0 match" nodes/elements
##.TimelineItem:if(#issue-1458171112)
#?#.TimelineItem[-ext-has='#issue-1458171112']
#?#.TimelineItem[-ext-has="#issue-1458171112"]

[-ext-has] usage:

  • AG Base: 123 lines, with domains +347 uses (15 disabled by comment for quora.com and 1 for wp.pl)
  • AG Mobile Ads: 23 lines, with domains 24 uses (0 disabled)
  • AG French: 6 lines/uses (0 disabled)
  • AG Spa/Por: 5 lines/uses (0 disabled)
  • AG Turkey: 36 lines/uses (0 disabled)
  • AG Annoyances: 24 lines, with domains +406 (0 disabled the most for google.* - legacy wrote many domains)
  • AG Social Media: 59 lines, with domains 73 uses (0 disabled)

:if(...) don't exist thankfully in AG files after convertion for uBO.

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

No branches or pull requests

3 participants