Opened on 08/29/2018 at 06:14:21 PM

Closed on 09/05/2018 at 06:54:33 AM

Last modified on 10/24/2018 at 02:21:07 PM

#6908 closed change (fixed)

Pass updated subscription's old filters as an event argument

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

https://codereview.adblockplus.org/29853570/

Description (last modified by mjethani)

Background

In the updateSubscriptionFilters function in lib/filterStorage.js, we stick the old filters to the Subscription object and later delete the property just so that the listener can have the list of old filters. This is a hack; we should just pass the old filters as an additional event argument.

What to change

In updateSubscriptionFilters in lib/filterStorage.js, pass the old filters of the subscription as an additional argument to the subscription.updated event. In lib/filterListener.js, accept and use this additional argument instead of the temporary oldFilters property of the Subscription object.

Integration notes

The subscription.updated event now includes the list of old filters as the second argument. Any listeners that assume that the argument will be undefined need to be updated. There is one case in adblockpluschrome in lib/requestBlocker.js.

Hints for testers

Subscribe to a filter list with filters A, B, and C, and make sure all three filters are working. Then remove filter B from the filter list and manually update the list on the extension's options page; now make sure only filters A and C are working.

Attachments (0)

Change History (5)

comment:1 Changed on 08/29/2018 at 06:16:39 PM by mjethani

  • Cc greiner sebastian added
  • Status changed from new to reviewing

comment:2 Changed on 09/04/2018 at 05:25:26 PM by abpbot

comment:3 Changed on 09/05/2018 at 06:54:33 AM by mjethani

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

comment:4 Changed on 09/05/2018 at 06:57:14 AM by mjethani

  • Description modified (diff)

comment:5 Changed on 10/24/2018 at 02:21:07 PM by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Working as described.

ABP 3.3.2.2175
Firefox 62 / 51 / Windows 10
Chrome 69 / 49 / Windows 10
Opera 56 / 36 / Windows 10

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 mjethani.
 
Note: See TracTickets for help on using tickets.