Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#2597 closed change (fixed)

[Move core logic into adblockpluscore repository] Change adblockplus dependency to adblockpluscore in adblockpluschrome

Reported by: kzar Assignee: kzar
Priority: P2 Milestone: Adblock-Plus-1.11-for-Chrome-Opera-Safari
Module: Platform Keywords:
Cc: fhd Blocked By: #2594
Blocking: #2593 Platform: Unknown
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29335668/

Description (last modified by fhd)

Background

Once we have successfully moved the core logic from adblockplus to adblockpluscore #2594 we need to replace the adblockplus dependency with adblockpluscore in the adblockpluschrome repository.

What to change

Replace the adblockplus dependency with adblockpluscore in the adblockpluschrome repository and ensure the required core code is still included.

Update: The adblockplus repository was left behind because some strings from it are still being used, #4275 is a follow-up for completely removing it.

Change History (18)

comment:1 Changed 4 years ago by kzar

  • Blocking 2593 added

comment:2 Changed 4 years ago by kzar

  • Blocked By 2594 added

comment:3 Changed 4 years ago by fhd

  • Component changed from Unknown to Platform
  • Tester set to Unknown

comment:4 Changed 4 years ago by fhd

  • Owner set to kzar

Assigning to kzar, since he's actually looking into that.

comment:5 Changed 4 years ago by fhd

  • Priority changed from Unknown to P2

comment:6 Changed 4 years ago by kzar

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

comment:7 Changed 4 years ago by kzar

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

comment:9 Changed 3 years ago by kzar

  • Owner kzar deleted

comment:10 Changed 3 years ago by fhd

kzar: Given how the push was apparently fixed by you five months ago, why not reassign yourself to this issue and then close it?

comment:11 Changed 3 years ago by kzar

We still import some strings from adblockplus. We eventually intend to move them all over to adblockplusui / their rightful place but until then we're keeping adblockplus as a dependency. I can't remember all the details, only that it was non-trivial to move them all.

(I unassigned myself from the issue since I'm not working on this right now and it's not a priority.)

comment:12 Changed 3 years ago by fhd

Fair enough, thanks for clarifying!

I suppose it's easier to understand if we separate the complete removal of the adblockplus dependency into a follow-up issue, I'll do that.

comment:13 Changed 3 years ago by kzar

Sounds good to me:)

adblockpluschrome/metadata.common has all the locale imports under the [import_locales] section. Here are the relevant lines:

adblockplus/chrome/locale/*/global.properties = subscription_invalid_location
  remove_subscription_warning
  notification_antiadblock_title
  notification_antiadblock_message
  filter_unknown_option
  filter_invalid_regexp
  filter_elemhide_duplicate_id
  filter_elemhide_nocriteria
  filter_cssproperty_nodomain
adblockplus/chrome/locale/*/overlay.dtd = hideplaceholders.label
  notification.button.yes
  notification.button.no
  notification.closing.button.hide
  notification.closing.button.optout
  shownotifications.label
adblockplus/chrome/locale/*/filters.dtd = subscription.lastDownload.inProgress
  subscription.lastDownload.invalidURL
  subscription.lastDownload.connectionError
  subscription.lastDownload.invalidData
  subscription.lastDownload.checksumMismatch
  subscription.enabled.label
  subscription.delete.label
  addSubscription.label
  addSubscriptionAdd.label
  addSubscriptionOther.label
  acceptableAds2.label
  viewList.label
  readMore.label
adblockplus/chrome/locale/*/subscriptionSelection.dtd = location.label
  title.label
adblockplus/chrome/locale/*/meta.properties = =name

comment:14 Changed 3 years ago by fhd

  • Description modified (diff)

There we go: #4275.

comment:15 Changed 3 years ago by fhd

Hehe, now we posted at almost the same time :) Would you mind adding that to #4275?

Last edited 3 years ago by fhd (previous) (diff)

comment:16 Changed 3 years ago by fhd

  • Owner set to kzar

Reassigning, since you were the one who actually did this.

comment:17 Changed 3 years ago by fhd

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:18 Changed 3 years ago by fhd

  • Milestone set to Adblock-Plus-1.11-for-Chrome-Opera-Safari
Note: See TracTickets for help on using tickets.