Opened on 04/12/2016 at 12:29:15 PM
Closed on 01/15/2018 at 02:48:46 PM
#3917 closed change (fixed)
Properly handle special subscriptions in new options page
| Reported by: | kzar | Assignee: | |
|---|---|---|---|
| Priority: | Unknown | Milestone: | |
| Module: | User-Interface | Keywords: | |
| Cc: | greiner, Ross, sebastian | Blocked By: | |
| Blocking: | Platform: | Unknown / Cross platform | |
| Ready: | no | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): | |||
Description (last modified by kzar)
Background
In #3915 Ross spotted that occasionally user added filters are wrongly displayed as subscriptions in the legacy options page. This regression was caused by the switch to asynchronous messaging. The same problem likely happens in the new options page as well.
What to change
Perform a similar fix to the new options page.
When the subscriptions.respond message with an action of added is received we need to first check if the subscription is a special one before handling it. If subscription.url starts with "~user" it's special and we must send a filters.get message for the subscription and add any filters given in the response. Otherwise we need to handle the new subscription as before.
Attachments (0)
Change History (6)
comment:1 Changed on 04/12/2016 at 12:37:35 PM by sebastian
comment:2 Changed on 04/13/2016 at 07:33:27 AM by kzar
- Resolution set to rejected
- Status changed from new to closed
comment:3 Changed on 04/14/2016 at 09:56:48 AM by kzar
- Blocked By 3915 removed
comment:4 Changed on 04/14/2016 at 10:00:02 AM by kzar
- Description modified (diff)
- Resolution rejected deleted
- Status changed from closed to reopened
comment:5 Changed on 04/29/2016 at 06:33:53 PM by greiner
According to adblockpluscore a SpecialSubscription is defined as a subscription with a URL that starts with ~.
Anyway, it'd be great if we could avoid duplicating this logic and instead determine that in the background page where we already know that it is a SpecialSubscription.
This issue shouldn't be blocked by #3915. The changes to the message responder arguably doesn't belong into a Platform issue in the first place. If it would be the only change required to adblockplusui it would be tolerable though. Also it's hard to review API changes without looking at the code integrating those anyway. So can you please create a patch including the changes to the message responder and the fix for the new options page for this issue, and mark it blocking #3915 instead.