Opened on 01/20/2017 at 02:46:26 PM
Closed on 08/29/2019 at 05:43:52 PM
#4828 closed change (rejected)
Move code, adding default subscriptions into adblockpluscore
| Reported by: | sergz | Assignee: | |
|---|---|---|---|
| Priority: | Unknown | Milestone: | |
| Module: | Core | Keywords: | closed-in-favor-of-gitlab |
| 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
Attachments (0)
Change History (7)
comment:1 Changed on 01/23/2017 at 01:45:59 PM by greiner
- Cc greiner added
comment:2 Changed on 12/21/2017 at 11:27:49 AM by fhd
- Cc trev removed
comment:3 Changed on 01/04/2018 at 01:22:37 PM by sergz
- Cc kzar added
comment:5 in reply to: ↑ 4 Changed on 01/08/2018 at 12:48:33 PM 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 on 05/23/2018 at 10:51:30 AM by mjethani
- Cc mjethani added
comment:7 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 new to closed
Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the new issue tracker.
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?