Opened on 03/30/2017 at 07:00:10 AM

Closed on 03/31/2017 at 08:47:24 AM

Last modified on 07/06/2017 at 01:08:41 PM

#5061 closed change (fixed)

Update adblockpluscore dependency to 3bdddf0e8343

Reported by: kzar Assignee: kzar
Priority: P2 Milestone: Adblock-Plus-1.13.3-for-Chrome-Opera
Module: Platform Keywords:
Cc: sebastian, trev Blocked By: #4878
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29398651/

Description (last modified by kzar)

Background

With #4878 we started using ESLint for the code in adblockpluscore. We want to update the dependency in adblockpluschrome to use those as soon as possible, they will require lots of testing.

As part of those changes we started using the alternative syntax of Cu.import.

For example this:

Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});

Became this:

const {XPCOMUtils} = Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});

I've also been asked to update the dependency past the ESLint changes, to include the most recent ones.

Full list of changes imported (irrelevant ones struck-through):

  1. Issue 4838 - Use nodeunit framework for integration tests running in browser
  2. Issue 4956 - Fix test_runner.js
  3. Issue 4962 - Fix the tests.
  4. Issue 4878 - Start using ESLint for adblockpluscore
  5. Noissue - Updated copyright year
  6. Issue 5037 - Look for "[-abp-properties=" to recognize the CSS filter
  7. Issue 4919 - Make getBackupFiles in lib/filterStorage.js asynchronous
  • The first three are related to the unit/browser tests in adblockpluscore and are irrelevant to adblockpluschrome.
  • The fourth is the ESLint changes themselves.
  • The fifth change is only to the copyright headers in all files and shouldn't change how adblockpluschrome behaves at all.
  • The sixth change is a small alteration to how CSS property filters are parsed.
  • The final change is to the getBackupFiles FilterStorage function which is only used by the Firefox extension, not adblockpluschrome.

What to change

  • Update the adblockpluscore dependency in adblockpluschrome to 3bdddf0e8343.
  • Update the Cu.import stub in lib/compat.js.

Hints for testers

  • Test CSS property filters still work properly.
  • The ESLint changes will need a thorough test of pretty much all code paths, on both a new and old version of Chrome. I recommend testing that at the same time as the changes for #4864.

Attachments (0)

Change History (7)

comment:1 Changed on 03/30/2017 at 07:00:20 AM by kzar

  • Ready set

comment:2 Changed on 03/30/2017 at 07:35:14 AM by kzar

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

comment:3 Changed on 03/30/2017 at 01:52:08 PM by kzar

  • Description modified (diff)
  • Summary changed from Update adblockpluscore dependency to 68a4dc4e79e8, including ESLint changes to Update adblockpluscore dependency to 3bdddf0e8343

comment:4 Changed on 03/30/2017 at 01:59:44 PM by kzar

  • Description modified (diff)

comment:5 Changed on 03/31/2017 at 08:46:30 AM by abpbot

A commit referencing this issue has landed:
Issue 5061 - Update adblockpluscore dependency to 3bdddf0e8343

comment:6 Changed on 03/31/2017 at 08:47:24 AM by kzar

  • Milestone set to Adblock-Plus-for-Chrome-Opera-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:7 Changed on 07/06/2017 at 01:08:41 PM by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. The extension overall is working as expected and CSS property filters work (apart from the issue with older browser versions, #5381).

ABP 1.13.2.1785
Chrome 51 / 59 / Windows 7
Opera 38 / 45 / Windows 7

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from kzar.
 
Note: See TracTickets for help on using tickets.