Opened 5 weeks ago

Last modified 3 weeks ago

#7360 new change

Provide interface to access contents of subscriptions.xml

Reported by: greiner Assignee:
Priority: Unknown Milestone:
Module: Core Keywords:
Cc: mjethani, sebastian, kzar, hfiguiere, jsonesen Blocked By: #7371
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description

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

Further information

Currently implemented at:

Change History (9)

comment:1 Changed 5 weeks 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 5 weeks ago by sebastian (previous) (diff)

comment:2 Changed 5 weeks 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 5 weeks 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 5 weeks ago by hfiguiere (previous) (diff)

comment:4 in reply to: ↑ 3 ; follow-up: Changed 5 weeks 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 5 weeks 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 5 weeks 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 5 weeks ago by sebastian

  • Blocked By 7371 added

comment:8 Changed 5 weeks ago by mjethani

  • Blocking 6977 added

comment:9 Changed 3 weeks ago by sebastian

  • Blocking 6977 removed
Note: See TracTickets for help on using tickets.