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 :has applies to multiple selectors; native :has only applies to the last one #2480

Closed
8 tasks done
antonok-edm opened this issue Feb 8, 2023 · 21 comments
Closed
8 tasks done
Labels
fixed issue has been addressed

Comments

@antonok-edm
Copy link

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 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

Cosmetic filters with multiple selectors and :has are interpreted differently depending on whether the browser has native :has support.

A specific URL where the issue occurs.

docs.rs

Steps to Reproduce

Add this custom filter:

docs.rs##form, .recent-releases-container:has(invalid)

Expected behavior

The filter should ideally be interpreted the same way on Chromium and Firefox.

Intuitively, form (which appears in the page's top navbar) should be hidden. As far as I can tell, the propagation of procedural operators to previously listed selectors is not documented in the wiki.

Actual behavior

The filter causes form to be hidden on Chromium browsers, but nothing is hidden in Firefox.

uBO version

1.46.0

Browser name and version

Firefox 109.0.1, Chromium 109.0.5414.87

Operating System and version

Arch Linux

@uBlock-user
Copy link
Contributor

uBlock-user commented Feb 8, 2023

but nothing is hidden in Firefox.

Support for :has() behind layout.css.has-selector.enabled in about:config.

@antonok-edm
Copy link
Author

antonok-edm commented Feb 8, 2023

@uBlock-user it's caused by the semantics uBO applies to procedural selectors vs. native selectors. uBO doesn't seem to be aware of the state of the about:config flag, so even with layout.css.has-selector.enabled set to true, the behavior is different.

In other words, without native :has, uBO treats the rule as equivalent to:

docs.rs##form:has(invalid)
docs.rs##.recent-releases-container:has(invalid)

versus on platforms with native :has, uBO treats it as equivalent to:

docs.rs##form
docs.rs##.recent-releases-container:has(invalid)

@antonok-edm
Copy link
Author

antonok-edm commented Feb 8, 2023

To be clear, I'm not sure if it's actually worth fixing this case specifically if Firefox is likely to catch up soon. But I'd definitely like to see the distinction between how native vs. non-native pseudoselectors are applied to multiple selectors:

  • either documented somewhere official
  • or simplified to always work like the native CSS pseudoselectors

because it's quite subtle and unexpected.

@gwarser
Copy link

gwarser commented Feb 8, 2023

  1. Concatenating multiple procedural selectors in one filter is not supported. example.com##p:has(img),div:has-text(advert) will not work as expected (#453).

https://github.com/uBlockOrigin/uBlock-issues/wiki/Procedural-cosmetic-filters#important

It was always broken, only slightly patched.

@garry-ut99

This comment was marked as abuse.

@krystian3w

This comment was marked as outdated.

@krystian3w
Copy link

Fix maybe need wait due is not easy:

#2228 (comment) #2228 (comment)

@MasterKia
Copy link
Member

MasterKia commented Feb 9, 2023

blog.onroid.com##.countdown, .get-link:others()

:others() is not implemented by AdGuard so it should be dropped AdGuard should drop those.

@krystian3w
Copy link

krystian3w commented Feb 9, 2023

#2480 (comment)

:others() is not implemented by AdGuard so it should be dropped.

Rather, it is useful - someone can do tests on how its presence affects a non-updated add-on (all line should be ignored). (AdguardTeam/ExtendedCss#164)

In part, it is worth watching to see if it begins to require use:

public.suffix##:is(a,b):others()
! or
public.suffix##:where(a,b):others()

When uBo copies the "CSS4" logic for all except vanilla selectors w/ & w/o :style only.

@krystian3w
Copy link

krystian3w commented Feb 9, 2023

To check also needed by use Orion:

https://browser.kagi.com/

Here vanilla CSS implementation should be unlocked if use same engine like Safari 15.4+.

@krystian3w

This comment was marked as outdated.

@garry-ut99

This comment was marked as abuse.

@u-RraaLL
Copy link
Contributor

@gorhill I'm on FF121 since a few hours ago, but it doesn't seem to work correctly with the :has() selector.

E.g. In this issue, before submitting this comment:

##p>code:first-of-type, div>pre 20 matches
##p>code:first-of-type, div:has(>pre) 5 matches - only selects div:has(>pre)
##p:has(>code:first-of-type), div>pre 0 matches
##p:has(>code:first-of-type), div:has(>pre) 0 matches

On Chrome it matched 20 for each of these. Correct elements/highlights.

Tested on a new browser profile for both.

@garry-ut99

This comment was marked as abuse.

@u-RraaLL
Copy link
Contributor

Maybe, but I thought this would fix it: gorhill/uBlock@c5724c1cce

@gorhill
Copy link
Member

gorhill commented Dec 25, 2023

Maybe, but I thought this would fix it: gorhill/uBlock@c5724c1cce

This is supposed to fix it, but for unknown reasons, runtime.getBrowserInfo is not defined in the element picker context, this causes native_css_has to never be set.

gorhill added a commit to gorhill/uBlock that referenced this issue Dec 25, 2023
@u-RraaLL u-RraaLL added the fixed issue has been addressed label Dec 25, 2023
@garry-ut99

This comment was marked as abuse.

@krystian3w
Copy link

krystian3w commented Dec 25, 2023

IMO 116+ are needed to safe hide elements, so ESR should be blocked untill released new 128 ESR at 9 june 2024 (and Windows 7, 8 and 8.1 moved to Mint or Ubuntu).

@krystian3w
Copy link

One could still add a warning in the linter that the filter can move in CSS 4 and will hide the element even if it has no children or siblings (e.g. lists from Nordic project https://github.com/DandelionSprout/adfilt if wasn't converted at all to use :is(..., ...):has(...)).

@garry-ut99

This comment was marked as abuse.

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

No branches or pull requests

8 participants