Opened 3 years ago

Closed 3 years ago

Last modified 2 years 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):

Description (last modified by kzar)


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

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 3 years ago by Ross

  • Description modified (diff)

comment:2 Changed 3 years 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 3 years 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 3 years ago by trev (previous) (diff)

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

  • Cc sebastian added
  • Description modified (diff)

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

  • Owner set to kzar

comment:10 Changed 3 years ago by kzar

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

comment:11 Changed 3 years ago by kzar

  • Cc greiner added

comment:12 Changed 3 years ago by kzar

  • Owner kzar deleted

Unassigning myself from this for now.

comment:13 Changed 3 years ago by mjethani

  • Owner set to mjethani

comment:14 Changed 3 years ago by mjethani

  • Review URL(s) modified (diff)

comment:15 Changed 3 years ago by abpbot

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

comment:16 Changed 3 years ago by saroyanm

  • Cc saroyanm added

comment:17 Changed 3 years ago by abpbot

comment:18 Changed 3 years ago by mjethani

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

comment:19 Changed 3 years ago by mjethani

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

comment:20 Changed 2 years ago by Ross

  • Verified working set

Fixed. Filters are no longer displayed twice.

Chrome 49 / 60 / Windows 10

Note: See TracTickets for help on using tickets.