Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29493605: Noissue - Eliminate CSS rules for whitelisted domains (Closed)

Created:
July 20, 2017, 3:29 p.m. by Manish Jethani
Modified:
July 25, 2017, 1:27 p.m.
Reviewers:
kzar
CC:
Sebastian Noack
Base URL:
https://hg.adblockplus.org/abp2blocklist
Visibility:
Public.

Description

Noissue - Eliminate CSS rules for whitelisted domains Consider the following filter list: ##.a foo.com##.b baz.com##.c @@||foo.com^$elemhide @@||bar.com^$elemhide @@||baz.com^$generichide @@||acceptable.baz.com^$elemhide Currently abp2blocklist generates the following rule set for this list: [ { "trigger": { "url-filter": "^https?://", "url-filter-is-case-sensitive": true, "unless-domain": [ "*baz.com", "*foo.com", "*bar.com", "*acceptable.baz.com" ] }, "action": { "type": "css-display-none", "selector": ".a" } }, { "trigger": { "url-filter": "^https?://([^/:]*\\.)?foo\\.com[/:]", "url-filter-is-case-sensitive": true, "unless-domain": [ "*foo.com", "*bar.com", "*acceptable.baz.com" ] }, "action": { "type": "css-display-none", "selector": ".b" } }, { "trigger": { "url-filter": "^https?://([^/:]*\\.)?baz\\.com[/:]", "url-filter-is-case-sensitive": true, "unless-domain": [ "*foo.com", "*bar.com", "*acceptable.baz.com" ] }, "action": { "type": "css-display-none", "selector": ".c" } } ] After this change, it will generate the following rule set: [ { "trigger": { "url-filter": "^https?://", "url-filter-is-case-sensitive": true, "unless-domain": [ "*baz.com", "*foo.com", "*bar.com", "*acceptable.baz.com" ] }, "action": { "type": "css-display-none", "selector": ".a" } }, { "trigger": { "url-filter": "^https?://([^/:]*\\.)?baz\\.com[/:]", "url-filter-is-case-sensitive": true, "unless-domain": [ "*acceptable.baz.com" ] }, "action": { "type": "css-display-none", "selector": ".c" } } ] What has changed: 1. There is no rule for foo.com since that domain is whitelisted 2. The unless-domain list for the rule for baz.com only includes the subdomain acceptable.baz.com This reduces the number of rules, but by a tiny amount with EasyList+AA. It is still the right thing to do. The main benefit is that it reduces the size of the unless-domain list for each CSS rule, which greatly reduces memory consumption and increases execution speed.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -9 lines) Patch
M lib/abp2blocklist.js View 4 chunks +18 lines, -7 lines 0 comments Download
M test/abp2blocklist.js View 1 chunk +3 lines, -2 lines 0 comments Download

Messages

Total messages: 3
Manish Jethani
July 20, 2017, 3:29 p.m. (2017-07-20 15:29:47 UTC) #1
Manish Jethani
Patch Set 1 Consider the following filter list: ##.a foo.com##.b baz.com##.c @@||foo.com^$elemhide @@||bar.com^$elemhide @@||baz.com^$generichide @@||acceptable.baz.com^$elemhide ...
July 20, 2017, 3:42 p.m. (2017-07-20 15:42:05 UTC) #2
kzar
July 25, 2017, 12:43 p.m. (2017-07-25 12:43:52 UTC) #3
LGTM - go ahead and push this one when you're ready.

Powered by Google App Engine
This is Rietveld