Opened 16 months ago

Closed 7 months ago

Last modified 5 months ago

#4329 closed change (fixed)

[abp2blocklist] Add (very rough) $generichide and $genericblock support

Reported by: kzar Assignee: mjethani
Priority: Unknown Milestone:
Module: Platform Keywords: abp2blocklist, AcceptableAds
Cc: sebastian, mario Blocked By:
Blocking: #4321, #5464 Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29439639/
https://codereview.adblockplus.org/29441592/

Description (last modified by mjethani)

Background

Currently abp2blocklist completely ignores $generichide and $genericblock filter options. While we cannot support them properly it might be possible to do something with them, hopefully reducing false positives. Content blocking rules have a limited unless-domain trigger field which we could perhaps use for some of our generic rules.

What to change

For $generichide exception filters, insert the rules between the generic element hiding rules and the domain-specific element hiding rules.

The rules should be in the following order:

  1. Generic element hiding rules (url-filter="^https?://", type="css-display-none")
  2. $generichide exceptions (url-filter="<specific_url>", type="ignore-previous-rules")
  3. Domain-specific element hiding rules (url-filter="^https?://<domain>", type="css-display-none")
  4. Other rules
  5. Other exceptions

The following filters:

##.ad
blogspot.com##div.textad
@@||googleblog.blogspot.com^$elemhide
@@||youtube.com^$generichide

Should give us the following rules, in this order:

[
  {
    "trigger": {
      "url-filter": "^https?://",
      "url-filter-is-case-sensitive": true
    },
    "action": {
      "type": "css-display-none",
      "selector": ".ad"
    }
  },
  { 
    "trigger": {
      "url-filter": "^https?://([^/]+\\.)?youtube\\.com",
      "url-filter-is-case-sensitive": true
    },
    "action": {
      "type": "ignore-previous-rules"
    }
  },
  {
    "trigger": {
      "url-filter": "^https?://([^/:]*\\.)?blogspot\\.com[/:]",
      "url-filter-is-case-sensitive": true
    },
    "action": {
      "type": "css-display-none",
      "selector": "div.textad"
    }
  },
  {
    "trigger": {
      "url-filter": "^https?://([^/]+\\.)?googleblog\\.blogspot\\.com",
      "url-filter-is-case-sensitive": true
    },
    "action": {
      "type": "ignore-previous-rules"
    }
  }
]

For $genericblock exception filters, take the hostname and put it into the unless-domain trigger for any generic blocking rules which don't already have an if-domain trigger.

The following filters:

_400x400.jpg|$image,domain=~facebook.com
@@||twitter.com/hello|$genericblock

Should give us the following rules:

[
  {
    "trigger": {
      "url-filter": "^https?://.*_400x400\\.jpg$",
      "resource-type": [
        "image"
      ],
      "unless-domain": [
        "*twitter.com",
        "*facebook.com"
      ]
    },
    "action": {
      "type": "block"
    }
  }
]

Notes

Limitations:

  • [genericblock] Adblock Plus exception filters will match the entire URL, whereas the unless-domain trigger will only match the hostname part, which means the rule will whitelist the entire domain instead of just the URL pattern
  • [generichide] In Adblock Plus filters $domain matches the domain of the parent document, whereas the unless-domain trigger matches the domain of the top-level document, which means the rule will not whitelist the domain if the document is not the top-level document

Change History (10)

comment:1 Changed 16 months ago by kzar

  • Description modified (diff)

comment:2 Changed 16 months ago by kzar

  • Component changed from Unknown to Platform

comment:3 Changed 7 months ago by mjethani

  • Owner set to mjethani

comment:4 Changed 7 months ago by mjethani

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:5 Changed 7 months ago by mjethani

  • Description modified (diff)

I had a discussion with Sebastian and Dave and we think we can do better at least with $generichide. I'm updating the description based on this, along with more specific details on how this should work.

comment:6 Changed 7 months ago by abpbot

A commit referencing this issue has landed:
Issue 4329 - Add $generichide support

comment:7 Changed 7 months ago by mjethani

  • Review URL(s) modified (diff)

comment:8 Changed 7 months ago by abpbot

A commit referencing this issue has landed:
Issue 4329 - Add $genericblock support

comment:9 Changed 7 months ago by mjethani

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:10 Changed 5 months ago by mjethani

  • Blocking 5464 added
Note: See TracTickets for help on using tickets.