Opened 7 months ago

Closed 3 months ago

Last modified 6 weeks ago

#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/
https://codereview.adblockplus.org/29479612/
https://codereview.adblockplus.org/29479615/

Description (last modified by kzar)

Environment

ABP 1.12.4.1739
Chrome 49 / 56 / Windows 10

How to reproduce

  1. Install Adblock Plus. (Remove and reinstall if already installed.)
  2. Open the Options Page, click Custom filters.
  3. Select [Edit as raw text].
  4. 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.

Change History (20)

comment:1 Changed 7 months ago by Ross

  • Description modified (diff)

comment:2 Changed 7 months ago by kzar

  • Cc kzar trev added
  • Component changed from Unknown to Platform

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.

comment:3 Changed 7 months ago 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.

Last edited 7 months ago by trev (previous) (diff)

comment:4 Changed 7 months ago 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 7 months ago by kzar

  • Cc sebastian added
  • Description modified (diff)

comment:6 Changed 7 months ago 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 7 months ago 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 7 months ago 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 7 months ago by kzar

  • Owner set to kzar

comment:10 Changed 7 months ago by kzar

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

comment:11 Changed 7 months ago by kzar

  • Cc greiner added

comment:12 Changed 6 months ago by kzar

  • Owner kzar deleted

Unassigning myself from this for now.

comment:13 Changed 4 months ago by mjethani

  • Owner set to mjethani

comment:14 Changed 4 months ago by mjethani

  • Review URL(s) modified (diff)

comment:15 Changed 4 months ago by abpbot

A commit referencing this issue has landed:
Issue 4978 - Include filters with special subscription

comment:16 Changed 4 months ago by saroyanm

  • Cc saroyanm added

comment:17 Changed 3 months ago by abpbot

comment:18 Changed 3 months ago by mjethani

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

comment:19 Changed 3 months ago by mjethani

  • Milestone set to Adblock-Plus-for-Chrome-Opera-next

comment:20 Changed 6 weeks ago by Ross

  • Verified working set

Fixed. Filters are no longer displayed twice.

ABP 1.13.3.1838
Chrome 49 / 60 / Windows 10

Note: See TracTickets for help on using tickets.