Opened on 05/22/2018 at 12:36:55 PM

Closed on 07/11/2018 at 04:48:25 PM

Last modified on 10/08/2019 at 06:05:04 PM

#6689 closed change (fixed)

Add anti-circumvention filter list to the recommended subscription list

Reported by: hfiguiere Assignee:
Priority: P2 Milestone:
Module: Core Keywords: circumvention
Cc: arthur, kzar, sebastian, mjethani Blocked By:
Blocking: #6538, #6699, #6781 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29824575/
https://codereview.adblockplus.org/29827630/

Description (last modified by hfiguiere)

Background

We want to have our own ABP filter list (mostly to deal with anti-circumvention). Therefore we want to have it in the recommended subscriptions list so that it can be easily enabled or disable by the user. See http://hub.eyeo.com/issues/11069 for the rationale.

Once the UI module is modified to allow having more filter lists (see https://gitlab.com/eyeo/adblockplus/adblockplusui/issues/85 - might require permission to view), we can supply that new subscription. (trac issue #6731)
An "Anti-Cirucumvention" section of the option page will allow enabling or disabling this filter list subscription.

The list of subscriptions is provided by adblockpluscore as chrome/content/ui/subscriptions.xml

See also http://hub.eyeo.com/issues/11271

What to change

  • Update the subscriptions.xml with a newer version, regenerated, that includes this new filter list subscription.
  • Add the optional type field to Subscription.

Note

adblockpluschrome will need to be updated to include both this change and the UI change. Also the subscription init code will need to be adapted to support more than one list: https://hg.adblockplus.org/adblockpluschrome/file/tip/lib/subscriptionInit.js#l202
(issue #6699)

Also this issue doesn't deal about having it enabled by default or when the extension is upgraded. This is will come at a later date (trac issue TBD)

Attachments (0)

Change History (30)

comment:1 Changed on 05/22/2018 at 12:45:39 PM by hfiguiere

  • Sensitive set

comment:2 Changed on 05/22/2018 at 12:45:58 PM by hfiguiere

  • Keywords circumvention added

comment:3 Changed on 05/22/2018 at 01:04:36 PM by hfiguiere

  • Cc kzar sebastian mjethani added

comment:4 Changed on 05/22/2018 at 01:06:26 PM by hfiguiere

  • Description modified (diff)
  • Summary changed from Add anti-cv filter list to Add anti-cv filter list to the recommended subscription list

comment:5 Changed on 05/23/2018 at 01:24:11 AM by mjethani

  • Description modified (diff)
  • Summary changed from Add anti-cv filter list to the recommended subscription list to Add anti-circumvention filter list to the recommended subscription list

comment:6 Changed on 05/23/2018 at 01:27:42 AM by mjethani

@hfiguiere can you add a little more detail to the ticket? What is this filter list supposed to be called (I'm assuming it's not literally called "anti-circumvention")? Is it going to be enough to update subscriptions.xml? I mean, what about the extension's UI, is it going to appear like a regular filter list or as a special list like Acceptable Ads with its own checkbox?

comment:7 Changed on 05/24/2018 at 01:24:54 PM by hfiguiere

The UI details are in https://gitlab.com/eyeo/adblockplus/adblockplusui/issues/85

We need a change in the UI to do this properly (this wasn't really known when I filed the issue).

Right now it will be called "ABP filters"

comment:8 follow-up: Changed on 05/24/2018 at 03:11:10 PM by kzar

That link isn't public apparently Hubert, also I agree with Manish that this ticket really does need more information. Like the last issue #6622 please add some background at the start as an introduction and work from there. For example please can you explain why do we want to have our own filter list?

comment:9 in reply to: ↑ 8 ; follow-ups: Changed on 05/24/2018 at 05:31:27 PM by hfiguiere

Replying to kzar:

That link isn't public apparently Hubert,

Much like this issue, it is marked as "confidential", and should be readable by anyone that have permission for adblockplusui on gitlab (I'm not sure who deal with that, definitely not me).

The tl;dr is that "MORE FILTERS" in the UI will be reworked to allow selecting this filter list (and others).

This issue is only about updating the subscription.xml in core after it gets regenerated. I need to file the issue for the WebExtension to update core + ui. (and I'm not sure yet how to submit to the subscriptionlist repository)

For example please can you explain why do we want to have our own filter list?

This is covered by http://hub.eyeo.com/issues/11069

comment:10 in reply to: ↑ 9 Changed on 05/24/2018 at 06:04:38 PM by kzar

Replying to hfiguiere:

Much like this issue, it is marked as "confidential"...

Fair enough, sorry I did not realise this issue was marked confidential.

This issue is only about updating the subscription.xml in core after it gets regenerated...

I understand that, but we still need a more useful issue description.

For example please can you explain why do we want to have our own filter list?

This is covered by http://hub.eyeo.com/issues/11069

Sorry, but I really am asking you to write proper issue descriptions here and going forward. Please can you start doing that.

I know it's a kind of a pain, but it's necessary. Until an issue has a proper description it cannot be triaged and therefore I end up spending a not insignificant amount of my time asking you to add information to your issues or simply giving up and adding it myself (after spending a bunch of time figuring out what it's about).

Code changes are really hard to review before the issue has been fleshed out as well. For example it took a lot of effort to get the description of #6622 into good shape - even though it related to the $rewrite option for which the changes you were chasing me to review. I couldn't even begin the reviews until I understood the context of what we were changing and why, so I had to first nag you to add more details. In the end I had to get clarification from Sebastian in IRC, I had no idea how $redirect and $rewrite were different.

Putting "Look at Hub issue X" in a comment is not enough. You need to put the required information and background into the Trac issue's description. If you're referencing other resources or issues, e.g. a Hub issue, then you should be linking them in the description too.

From the TracGuide;

Don't bury the description in the comments
Don't put any part of the ticket into a comment. Instead change the description and comment your change. The comments are not part of the ticket and all additional parts of the ticket (priority, stage, etc....) don't and cannot apply to the comments.

Imagine the reader has no idea about the plan to add this new filter list, why we're adding it, or where they can go to read more about it. If you'd start the description off with a brief introduction aimed at them it would go a long way.

Last edited on 05/24/2018 at 06:10:12 PM by kzar

comment:11 Changed on 05/24/2018 at 10:24:12 PM by hfiguiere

  • Description modified (diff)

Updated the description. There are a few related trac issues that are currently marked as TBD. I'll be filling these gaps ASAP.

comment:12 in reply to: ↑ 9 Changed on 05/25/2018 at 08:12:46 AM by arthur

Replying to hfiguiere:

and I'm not sure yet how to submit to the subscriptionlist repository)

I'm taking care of that usually.

comment:13 Changed on 05/25/2018 at 03:00:00 PM by hfiguiere

  • Blocked By 6699 added

comment:14 Changed on 05/25/2018 at 03:02:21 PM by hfiguiere

  • Description modified (diff)

comment:15 Changed on 05/25/2018 at 03:16:23 PM by hfiguiere

  • Blocked By 6699 removed
  • Blocking 6699 added

comment:16 Changed on 05/29/2018 at 11:48:56 AM by kzar

  • Description modified (diff)

Thanks Hubert, the issue is looking way better now.

comment:17 Changed on 05/29/2018 at 12:21:08 PM by hfiguiere

  • Description modified (diff)

comment:18 Changed on 06/13/2018 at 01:57:39 PM by hfiguiere

  • Description modified (diff)

comment:19 Changed on 07/06/2018 at 01:50:13 PM by mjethani

  • Priority changed from Unknown to P2
  • Ready set

comment:20 Changed on 07/06/2018 at 02:29:50 PM by mjethani

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

comment:21 Changed on 07/06/2018 at 03:27:03 PM by mjethani

  • Blocking 6538 added

comment:22 Changed on 07/09/2018 at 07:59:13 AM by abpbot

A commit referencing this issue has landed:
Issue 6689 - Add type property to Subscription class

comment:23 Changed on 07/11/2018 at 03:08:45 PM by hfiguiere

  • Review URL(s) modified (diff)

comment:24 Changed on 07/11/2018 at 04:47:37 PM by abpbot

A commit referencing this issue has landed:
Issue 6689 - Added anti-CV filter list subscription

comment:25 Changed on 07/11/2018 at 04:48:25 PM by hfiguiere

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

comment:26 Changed on 07/11/2018 at 07:31:01 PM by hfiguiere

  • Sensitive unset

comment:27 Changed on 07/20/2018 at 12:57:56 PM by mjethani

  • Blocking 6781 added

comment:28 Changed on 08/07/2018 at 08:24:07 AM by Ross

Done. Tested with #6783.

ABP 3.2.2102
Chrome 66 / 55 / 49 / Windows 10
Firefox 61 / 55 / 51

comment:29 Changed on 01/11/2019 at 07:31:13 AM by mayduavongts

spam

Last edited on 10/08/2019 at 06:05:01 PM by kzar

comment:30 Changed on 03/21/2019 at 04:52:08 AM by nhaphangali

spam

Last edited on 10/08/2019 at 06:05:04 PM by kzar

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
 
Note: See TracTickets for help on using tickets.