Opened 9 months ago

Last modified 5 months ago

#7360 closed change

Provide interface to access contents of subscriptions.xml — at Version 10

Reported by: greiner Assignee:
Priority: P2 Milestone:
Module: Core Keywords:
Cc: mjethani, sebastian, kzar, hfiguiere, jsonesen 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/74

Description (last modified by mjethani)

Background

We're fetching, parsing and interpreting the contents of subscriptions.xml in various places throughout our code (adblockpluschrome/subscriptionInit.js, adblockplusui/js/desktop-options.js, adblockplusui/mobile-options.js) and with ui#9, we'd do that again.

While we could share that code across adblockplusui or even adblockpluschrome, this seems to be something that's relevant for anyone who has Core as a dependency. Also, discouraging non-Core code from directly accessing subscriptions.xml would allow Core to assert more control over how it's being used.

What to change

Expose API for accessing information contained within subscriptions.xml from outside Core

Introduce a new recommendations({types} = {}) generator function in a new file lib/recommendations.js that yields the subscriptions in data/subscriptions.json. The first and only parameter represents the match criteria: if types is not undefined, it must be an array containing values like "ads", "circumvention", etc., and the function should yield only those entries that have a matching type.

Also add a test/recommendations.js.

Further information

Currently implemented at:

Change History (10)

comment:1 Changed 9 months ago by sebastian

  • Cc kzar added

Also note that with manifest V3 we no longer have access to DOMParser in the background page but are limited to the APIs available in Service Workers (plus extension APIs). So it might be a good time to change the format of subscription.xml to JSON.

Last edited 9 months ago by sebastian (previous) (diff)

comment:2 Changed 9 months ago by mjethani

  • Cc hfiguiere jsonesen added

@kzar thanks for pointing out about DOMParser

Would it be enough for the three use cases to simply provide a subscriptions.json instead?

Also I am wondering if we should move the file to the data directory in adblockpluscore now.

comment:3 follow-up: Changed 9 months ago by hfiguiere

This involve changing the sitescripts to generate JSON instead of XML from the subscriptionlist (an issue should be filed). Moving it to the data directory would make sense.

Last edited 9 months ago by hfiguiere (previous) (diff)

comment:4 in reply to: ↑ 3 ; follow-up: Changed 9 months ago by sebastian

Replying to mjethani:

@kzar thanks for pointing out about DOMParser

That was me. ;)

Also I am wondering if we should move the file to the data directory in adblockpluscore now.

Agreed.

Replying to hfiguiere:

This involve changing the sitescripts to generate JSON instead of XML from the subscriptionlist (an issue should be filed). Moving it to the data directory would make sense.

Please correct me if I'm wrong, but IIRC that automation has been inactive for years. Either way, we should replace it with an npm script within adblockpluscore, and if necessary automate it through GitLab CI (it supports periodical jobs).

comment:5 in reply to: ↑ 4 Changed 9 months ago by hfiguiere

Replying to sebastian:

Please correct me if I'm wrong, but IIRC that automation has been inactive for years. Either way, we should replace it with an npm script within adblockpluscore, and if necessary automate it through GitLab CI (it supports periodical jobs).

I have been running these scripts each and every time I updated the .xml file.
But yes we can replace this with something less obscure.

comment:6 Changed 9 months ago by mjethani

Let's open a separate ticket for the work in the Automation module for generating a subscriptions.json in the data directory in adblockpluscore. Let's keep this one for any further work in adblockpluscore for providing an API over the subscriptions.json.

There are two options: adblockpluschrome and adblockplusui use the subscriptions.json file directly by require()'ing it, or adblockpluscore provides an API (e.g. subscriptions.query({prefixes: ["es"]})).

comment:7 Changed 9 months ago by sebastian

  • Blocked By 7371 added

comment:8 Changed 9 months ago by mjethani

  • Blocking 6977 added

comment:9 Changed 8 months ago by sebastian

  • Blocking 6977 removed

comment:10 Changed 7 months ago by mjethani

  • Description modified (diff)
  • Priority changed from Unknown to P2
  • Ready set
Note: See TracTickets for help on using tickets.