Opened on 03/13/2017 at 09:17:42 AM
Closed on 07/17/2017 at 03:45:15 PM
Last modified on 09/07/2017 at 01:43:38 PM
#4978 closed defect (fixed)
Custom filters edit as raw text sometimes duplicates entered filters
| Reported by: | Ross | Assignee: | mjethani |
|---|---|---|---|
| Priority: | P2 | Milestone: | Adblock-Plus-1.13.4-for-Chrome-Opera |
| Module: | Platform | Keywords: | |
| Cc: | kzar, trev, sebastian, greiner, saroyanm | Blocked By: | |
| Blocking: | Platform: | Chrome | |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): |
https://codereview.adblockplus.org/29384555/ |
||
Description (last modified by kzar)
Environment
ABP 1.12.4.1739
Chrome 49 / 56 / Windows 10
How to reproduce
- Install Adblock Plus. (Remove and reinstall if already installed.)
- Open the Options Page, click Custom filters.
- Select [Edit as raw text].
- Paste the following filters and select [Apply changes].
#@#.da_adp_teaser
#@#.directadvert-block
sibnet.ru#@#.header__topline
Observed behaviour
Several of the filters are listed multiple times. When the options page is refreshed the duplicates are no longer displayed.
Expected behaviour
None of the added filters should be listed multiple times.
Attachments (0)
Change History (20)
comment:2 Changed on 03/13/2017 at 10:17:13 AM by kzar
- Cc kzar trev added
- Component changed from Unknown to Platform
comment:3 Changed on 03/13/2017 at 10:21:08 AM by trev
I can reproduce the issue with a clean install (development environment).
Edit: The filters are actually fine, they are merely not being displayed correctly. If you reload the page the duplicate filters will be gone.
comment:4 Changed on 03/13/2017 at 10:34:22 AM by kzar
- Description modified (diff)
- Priority changed from Unknown to P2
- Ready set
Ah OK, after I removing and reinstalling the extension I can also reproduce this. The filter doesn't seem to be added twice, but the textbox is displaying some twice for some reason.
I could also reproduce for 1.12.4, so I don't think this should be a release blocker.
comment:5 Changed on 03/13/2017 at 10:36:28 AM by kzar
- Cc sebastian added
- Description modified (diff)
comment:6 Changed on 03/13/2017 at 10:47:07 AM by trev
This is a regression from #3870 which landed in 1.12. We have three messages being dispatched here, one "subscription.added" (dispatched when the first filter is added) and two "filter.added". The issue is, by the time "subscription.added" is processed the other two filters have been added to this subscription as well because messages are dispatched asynchronously.
The root of the issue is that the options page is getting the real subscription object here for some reason which is still changing, as opposed to a structured clone of it which would be fixed.
comment:7 Changed on 03/13/2017 at 10:52:15 AM by trev
Actually, it isn't the real subscription object. However, the structured clone has no filters on it so the options page requests them asynchronously - and that's where it gets data which is newer than the notification.
comment:8 Changed on 03/13/2017 at 10:57:18 AM by kzar
Yea, I think this is since when the extension is freshly installed we don't yet have a subscription for custom filters. So when the first filter is added the options page receives both a "subscriptions.respond" message and a "filters.respond" one. This ends up meaning that filter is displayed twice in the list.
comment:9 Changed on 03/13/2017 at 11:04:14 AM by kzar
- Owner set to kzar
comment:10 Changed on 03/15/2017 at 07:26:21 AM by kzar
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:11 Changed on 04/03/2017 at 12:45:34 PM by kzar
- Cc greiner added
comment:12 Changed on 04/28/2017 at 09:17:29 AM by kzar
- Owner kzar deleted
Unassigning myself from this for now.
comment:13 Changed on 07/04/2017 at 01:42:28 PM by mjethani
- Owner set to mjethani
comment:14 Changed on 07/04/2017 at 02:36:31 PM by mjethani
- Review URL(s) modified (diff)
comment:15 Changed on 07/05/2017 at 11:51:48 AM by abpbot
A commit referencing this issue has landed:
Issue 4978 - Include filters with special subscription
comment:16 Changed on 07/05/2017 at 12:23:33 PM by saroyanm
- Cc saroyanm added
comment:17 Changed on 07/17/2017 at 03:42:52 PM by abpbot
A commit referencing this issue has landed:
Issue 4978 - Read special subscription filters directly in options page
comment:18 Changed on 07/17/2017 at 03:45:15 PM by mjethani
- Resolution set to fixed
- Status changed from reviewing to closed
comment:19 Changed on 07/18/2017 at 04:59:09 AM by mjethani
- Milestone set to Adblock-Plus-for-Chrome-Opera-next
comment:20 Changed on 09/07/2017 at 01:43:38 PM by Ross
- Verified working set
Fixed. Filters are no longer displayed twice.
ABP 1.13.3.1838
Chrome 49 / 60 / Windows 10
I can't reproduce this so far, going to try on my Windows 10 VM now.
Can you confirm if the problem also happens for you with 1.12.4? If so it's not a regression we need to worry about before the 1.13 release.