Opened 21 months ago

Closed 3 days ago

#3143 closed change (fixed)

Add polyfill for `:has()` CSS4 pseudo-selector to element hiding filters

Reported by: greiner Assignee: hfiguiere
Priority: P2 Milestone:
Module: Core Keywords:
Cc: mapx, Lain_13, sebastian, fhd, mario, kzar, rach, arthur, sergz, oleksandr Blocked By: #4394, #4726, #4796, #4962, #5287
Blocking: #2360, #5094, #5249, #5313 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29383960/
https://codereview.adblockplus.org/29464696/

Description (last modified by trev)

Background

(taken from original description in #2360 about hiding elements by selecting their child elements)

Currently, there's no way to hide an element on a page dependent on its children. This proposed solution is based on existing CSS functionality, so it will be possible to bind an observer to the document to check for new parent nodes and bind another observer object to them to check for changes within only these particular nodes in case specific child node will be added. Over time it will be possible to completely disable such functionality due to native support for CSS4 selectors in newer browser versions.

It may be possible to look at the jQuery code for this functionality since they supported :has() pseudo-selector in their selectors long before it was even added into the CSS4 drafts.

Generally, this proposal has similar issues as CSS property filters such as requiring to hide elements manually and to observe the page for changes at runtime.

:has() and CSS property filters have to work in conjunction, therefore they need to be stored in the same filter class (see #4394). In addition, we will probably have to change the way CSS property filters work: Currently we eventually generate standard CSS rules for them, which are then being applied by the browser. For :has(), we have to apply the rules manually.

What to change

  • A new special pseudo-classe :-abp-has(...) should be added. Here, the parameter is a selector - this pseudo-class should only match elements where the inner selector produces any hits.
  • It should be possible to nest pseudo-classes, in particular :-abp-has(:-abp-properties(...)) has to be supported.
  • ElemHideEmulation class should get a new removeElements callback, to be called for any elements selected by filters with :-abp-has() pseudo-class. Filters only using :-abp-properties() should use addSelectors callback as before.

Optional changes

  • Allow :-abp-has() pseudo-classes to be nested.
  • Observe DOM changes to apply filters to dynamically added/modified elements

References

Change History (41)

comment:1 Changed 21 months ago by greiner

  • Type changed from defect to change

comment:2 Changed 21 months ago by greiner

  • Blocking 2360 added

comment:3 Changed 21 months ago by mapx

  • Cc mapx Lain_13 added

comment:4 Changed 12 months ago by fhd

  • Owner set to fhd

I'll have a go at this one.

comment:5 Changed 12 months ago by fhd

  • Priority changed from Unknown to P2

comment:6 follow-up: Changed 11 months ago by Lain_13

I think it's worth mentioning that :has() selector must work in combination with -abp-properties. So, filter like site.name##.block:has([-abp-properties="background: yellow"]) must hide all elements of class "block" with anything with word "yellow" in a "background" property inside of them.

Last edited 11 months ago by Lain_13 (previous) (diff)

comment:7 Changed 10 months ago by kzar

  • Cc sebastian fhd added
  • Owner changed from fhd to kzar

comment:8 Changed 10 months ago by kzar

  • Description modified (diff)

comment:9 Changed 10 months ago by mapx

"waiting for #2388 to be closed probably makes the most sense to get the necessary architecture in place first"

#2388 ==> still opened #2398 ==> still opened #2402 (unit tests)

(fhd comment: Given how CSS property filters support has already landed in Firefox - how about closing this one (and #2388) and removing #2402 as a dependency? )

comment:10 Changed 10 months ago by kzar

  • Blocked By 2388 removed

Yea I think you're right, I don't think this is blocked by #2388 any more. I'll remove that as blocking now.

comment:11 in reply to: ↑ 6 Changed 10 months ago by mapx

Replying to Lain_13:

I think it's worth mentioning that :has() selector must work in combination with -abp-properties. So, filter like site.name##.block:has([-abp-properties="background: yellow"]) must hide all elements of class "block" with anything with word "yellow" in a "background" property inside of them.

it's exactly the last trick FB used to defeat the various userscripts (using css ::after)

comment:12 Changed 10 months ago by kzar

  • Cc mario added

comment:13 Changed 10 months ago by kzar

  • Cc kzar added
  • Owner kzar deleted

(I have had nearly no chance to look at this so far and am on vacation next week. Will reassign myself again when I have time if no one else has picked it up.)

comment:14 Changed 10 months ago by ethaniel

Can't wait for this ticket to be resolved. Yandex is using every possibility to evade the block. Still seeing ads.

comment:15 Changed 10 months ago by fhd

  • Owner set to fhd

With Dave gone for a while, I'll pick this up again, this time for real :)

I guess I still won't find enough time to finish it, but there's some ground work to be done I can get out of the way at the least, I'll create a blocking issue for that soon.

comment:16 Changed 10 months ago by fhd

  • Blocked By 4394 added

comment:17 Changed 10 months ago by fhd

  • Description modified (diff)

I've created #4394 for the ground work I was talking about. I'll take care of that first.

I realised that in order for :has() and CSS property filters to work in conjunction (which I also see as a must), we'll have to change the way CSS property filters are being applied somewhat fundamentally. I've updated the description a bit to reflect that.

comment:18 Changed 9 months ago by rach

  • Cc rach added

comment:19 Changed 7 months ago by fhd

  • Blocked By 4658 added

comment:20 Changed 7 months ago by fhd

  • Blocked By 4658 removed

comment:21 Changed 7 months ago by arthur

  • Cc arthur added

comment:22 Changed 6 months ago by fhd

  • Blocked By 4726 added

comment:23 Changed 4 months ago by hfiguiere

  • Blocked By 4962 added

comment:24 Changed 3 months ago by hfiguiere

  • Review URL(s) modified (diff)

comment:25 Changed 3 months ago by fhd

  • Owner changed from fhd to hfiguiere

comment:26 Changed 3 months ago by sergz

  • Cc sergz added

comment:27 Changed 3 months ago by hfiguiere

  • Status changed from new to reviewing

comment:28 Changed 3 months ago by hfiguiere

  • Blocking 5094 added

comment:29 Changed 2 months ago by oleksandr

  • Cc oleksandr added

comment:30 Changed 2 months ago by trev

  • Blocked By 4796 added

comment:31 Changed 5 weeks ago by hfiguiere

  • Blocking 5249 added

comment:32 Changed 3 weeks ago by trev

  • Description modified (diff)
  • Ready set

comment:33 Changed 3 weeks ago by trev

  • Blocked By 5287 added

comment:34 Changed 3 weeks ago by trev

  • Description modified (diff)

I updated the description, the syntax modification is now #5287.

comment:35 Changed 3 weeks ago by Lain_13

After recent changes in the description I'd like to clarify one thing:
Will ABP support :has() as originally were requested or only :-abp-has()? uBO already supports :has() and it would be nice to have compatibility between ABP and uBO from the get-go.

comment:36 Changed 3 weeks ago by hfiguiere

Right now it is settled on :-abp-has() and the new filter syntax specified in #5287. :has() is not be supported.

comment:37 Changed 9 days ago by hfiguiere

  • Blocking 5313 added

comment:38 Changed 9 days ago by abpbot

A commit referencing this issue has landed:
Issue 3143 - Filter elements with :-abp-has()

comment:39 Changed 9 days ago by hfiguiere

  • Review URL(s) modified (diff)

comment:40 Changed 3 days ago by abpbot

A commit referencing this issue has landed:
Issue 3143 - Remove internal tests

comment:41 Changed 3 days ago by hfiguiere

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.