Opened 11 months ago

Last modified 5 weeks ago

#6977 reviewing change

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

Reported by: mjethani Assignee: mjethani
Priority: P2 Milestone:
Module: Core Keywords:
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.

Change History (16)

comment:1 Changed 11 months ago by mjethani

  • Cc hfiguiere sergz added

comment:2 Changed 11 months ago by mjethani

  • Description modified (diff)

comment:3 Changed 11 months ago by mjethani

  • Cc kzar added

comment:4 Changed 11 months ago by sebastian

  • Cc sebastian added

comment:5 Changed 11 months ago by mjethani

  • Cc greiner added

comment:6 Changed 5 months ago by mjethani

  • Blocked By 7360 added

comment:7 Changed 5 months ago by sebastian

  • Blocked By 7371 added; 7360 removed

comment:8 Changed 5 months ago 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 5 months ago by mjethani

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

comment:10 Changed 5 months ago by tlucas

  • Cc tlucas added

comment:11 Changed 3 months ago 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 3 months ago by mjethani

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

comment:13 Changed 3 months ago by mjethani

  • Description modified (diff)

comment:14 Changed 3 months ago by mjethani

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

comment:15 Changed 3 months ago by abpbot

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

comment:16 Changed 5 weeks ago 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

Note: See TracTickets for help on using tickets.