Opened on 09/24/2018 at 01:58:16 PM

Closed on 08/29/2019 at 05:43:52 PM

#6977 closed change (rejected)

Detect anti-circumvention filter list based on URL in subscriptions.json

Reported by: mjethani Assignee: mjethani
Priority: P2 Milestone:
Module: Core Keywords: closed-in-favor-of-gitlab
Cc: hfiguiere, sergz, kzar, sebastian, greiner, tlucas Blocked By: #7371
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://gitlab.com/eyeo/adblockplus/adblockpluscore/merge_requests/79

Description (last modified by mjethani)

Background

See #6974.

#6975 is a temporary fix for this issue just for the 3.4 release. There can be multiple anti-circumvention lists, it's not feasible to keep all the URLs hardcoded. Instead, it would be better to somehow get the URLs out of the subscriptions.json file, which is already part of core.

What to change

In Subscription.fromURL() in lib/subscriptionClasses.js, assign the type property automatically based on the data in data/subscriptions.json.

Attachments (0)

Change History (17)

comment:1 Changed on 09/24/2018 at 01:58:47 PM by mjethani

  • Cc hfiguiere sergz added

comment:2 Changed on 09/24/2018 at 02:36:37 PM by mjethani

  • Description modified (diff)

comment:3 Changed on 09/24/2018 at 02:36:54 PM by mjethani

  • Cc kzar added

comment:4 Changed on 09/24/2018 at 03:00:41 PM by sebastian

  • Cc sebastian added

comment:5 Changed on 09/24/2018 at 03:13:13 PM by mjethani

  • Cc greiner added

comment:6 Changed on 03/16/2019 at 04:04:02 PM by mjethani

  • Blocked By 7360 added

comment:7 Changed on 03/30/2019 at 03:00:48 AM by sebastian

  • Blocked By 7371 added; 7360 removed

comment:8 Changed on 04/01/2019 at 10:22:17 AM by greiner

Note that adblockplusui already relies on there only being one filter list of each type in subscriptions.xml (except for type=ads) so that it can uniquely associate them to the various UI elements that en-/disable them. Therefore we could either close this ticket since that assumption already exists in production or we could add a "recommended" flag to each <subscription> element that won't be set for any additional filter lists we're going to add later on (see also ui#196).

On another note, it might be a good idea to automatically validate that subscriptions.xml doesn't violate any of our assumptions. While we could do that in adblockplusui, adblockpluscore might be a better place for such a linter for the time being.

comment:9 Changed on 04/01/2019 at 03:18:14 PM by mjethani

About the validation, I have suggested it in ticket:7371#comment:16.

comment:10 Changed on 04/02/2019 at 08:02:31 AM by tlucas

  • Cc tlucas added

comment:11 Changed on 05/23/2019 at 08:31:20 AM by mjethani

  • Description modified (diff)
  • Owner set to mjethani
  • Ready set
  • Summary changed from Detect anti-circumvention filter list based on URL in subscriptions.xml to Detect anti-circumvention filter list based on URL in subscriptions.json

comment:12 Changed on 05/23/2019 at 08:32:25 AM by mjethani

We know how to do this now but I'm waiting for core!74 to land.

comment:13 Changed on 05/23/2019 at 08:33:30 AM by mjethani

  • Description modified (diff)

comment:14 Changed on 05/23/2019 at 08:44:05 AM by mjethani

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

comment:15 Changed on 05/25/2019 at 07:40:19 AM by abpbot

A commit referencing this issue has landed:
Issue 6977 - Auto-detect subscription type based on URL

comment:16 Changed on 07/24/2019 at 12:46:15 PM by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Subscriptions types look to still be behaving as expected.

ABP 0.9.15.2340
Microsoft Edge 44.17763.1.0 / Windows 10 1809

ABP 3.5.2.2340
Chrome 49.0.2623.75 / Windows 10 1809
Chrome 75.0.3770.142 / Windows 10 1809
Opera 36.0.2130.65 / Windows 10 1809
Opera 62.0.3331.72 / Windows 10 1809
Firefox 51.0 / Windows 10 1809
Firefox 68.0 / Windows 10 1809
Firefox Mobile 68.0 / Android 7.2.2

comment:17 Changed on 08/29/2019 at 05:43:52 PM by sebastian

  • Keywords closed-in-favor-of-gitlab added
  • Resolution set to rejected
  • Status changed from reviewing to closed

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the new issue tracker.

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 mjethani.
 
Note: See TracTickets for help on using tickets.