Opened 16 months ago

Closed 15 months ago

Last modified 14 months ago

#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.

Change History (5)

comment:1 Changed 16 months ago by mjethani

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

comment:2 Changed 15 months ago by abpbot

comment:3 Changed 15 months ago by mjethani

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

comment:4 Changed 15 months ago by mjethani

  • Description modified (diff)

comment:5 Changed 14 months ago 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

Note: See TracTickets for help on using tickets.