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

Issue 29336748: Issue 3671 - Expose getBaseDomain for abp2blocklist (Closed)

Feb. 20, 2016, 3:29 p.m. by kzar
Feb. 21, 2016, 8:10 a.m.
Sebastian Noack


Issue 3671 - Expose getBaseDomain for abp2blocklist

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -2 lines) Patch
M lib/url.js View 2 chunks +9 lines, -2 lines 0 comments Download


Total messages: 3
Patch Set 1
Feb. 20, 2016, 10:01 p.m. (2016-02-20 22:01:38 UTC) #1
Sebastian Noack
What is if it's an IP address? That is a case intentionally ignored in getBaseDomain() ...
Feb. 21, 2016, 1:42 a.m. (2016-02-21 01:42:27 UTC) #2
Feb. 21, 2016, 8:09 a.m. (2016-02-21 08:09:44 UTC) #3
On 2016/02/21 01:42:27, Sebastian Noack wrote:
> Moreover, as abp2blocklist isn't used yet in adblockpluschrome, I'm not too
> happy that you attached this review to the same issue.

Well I was trying to make the minimum changes required so that we could generate
a content blocking list from both the Node.js script and from the extension.
(Without any changes to contentBlockerLists.js.) But I see your point, we're not
actually using content blocker lists from this extension yet so I guess we can
just worry about this change later. I'll close this for now.

Powered by Google App Engine
This is Rietveld