Opened 3 years ago

Last modified 15 months ago

#4828 new change

Move code, adding default subscriptions into adblockpluscore

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

Description (last modified by sergz)

Background

We have several implementations in different places of one functionality which does the same thing - it adds default subscriptions, in particular, including acceptable ads, on the first run.
Firefox: https://github.com/adblockplus/adblockplus/blob/master/lib/ui.js#L803
Chrome: https://github.com/adblockplus/adblockpluschrome/blob/e1da5f862b2f0548b1e222525bfe18cdb44dd4e9/lib/subscriptionInit.js#L150
Libadblockplus: https://github.com/adblockplus/libadblockplus/blob/72dd22b538b27b46046a42d670e909f5f332ccf2/lib/init.js#L60
Internet Explorer: https://github.com/adblockplus/adblockplusie/blob/847b062636bf5ed9517f8e78d448a543c1d466f4/src/plugin/PluginClass.cpp#L754

What to change

  • align with new options page and check that there is no such logic in adblockplusui repository
  • add a unified implementation into adblockpluscore
  • make changes in the mentioned above projects

Change History (6)

comment:1 Changed 3 years ago by greiner

  • Cc greiner added

comment:2 Changed 20 months ago by fhd

  • Cc trev removed

comment:3 Changed 20 months ago by sergz

  • Cc kzar added

comment:4 follow-up: Changed 20 months ago by kzar

Well unless I'm mistaken both adblockplusie and adblockplus are deprecated now, so we'd just be removing the duplication between adblockpluschrome and libadblockplus. (Also since those links are using the master bookmark the line numbers are wrong now.) Would you mind updating the description?

comment:5 in reply to: ↑ 4 Changed 20 months ago by sergz

  • Cc wspee added
  • Description modified (diff)

Replying to kzar:

Well unless I'm mistaken both adblockplusie and adblockplus are deprecated now, so we'd just be removing the duplication between adblockpluschrome and libadblockplus. (Also since those links are using the master bookmark the line numbers are wrong now.) Would you mind updating the description?

The links are updated but the links for FF and IE are left because of the following reasons. In FF the implementation significantly differs from the implementation in other projects and it's not clear what should be chosen. The status of IE is not finally deprecated, so it still can happen that Oleksandr decides to update libadblockplus in IE at some point.

I have also added @wspee into CC in order to firstly synchronize the desired behaviour with new options page, @wspee could you please tell what subscriptions, when and how should be added?

comment:6 Changed 15 months ago by mjethani

  • Cc mjethani added
Note: See TracTickets for help on using tickets.