Opened on 07/29/2014 at 03:30:30 PM

Closed on 08/29/2019 at 05:43:52 PM

Last modified on 10/08/2019 at 05:25:34 PM

#1162 closed change (rejected)

Cache notification URL matcher

Reported by: greiner Assignee: greiner
Priority: P2 Milestone:
Module: Core Keywords: closed-in-favor-of-gitlab
Cc: Blocked By: #7432
Blocking: Platform: Unknown
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by greiner)


We introduced notifications that can be triggered when matching a given set of filters for the anti-adblock notification. In the current implementation we are instantiating the matcher each time when we check for a notification based on the assumption that the performance impact would be insignificant.

However, since this is not the case due to this happening on each request we should only instantiate it once and reuse it after that.

What to change

In lib/notification.js:

  • Instantiate the matcher on notification creation and attach it to the notification as a property _matcher
  • Notifications should remain serializable (replace JSON.serialize() to drop all properties starting with "_" if necessary)
  • Remove the instantiation in getNextToShow() and instead use the matcher associated with the notification

Hints for testers

  • Check that there are no errors after installing the extension
  • Check that anti-adblock notification shows up on domains listed in the filter list and doesn't show up for any other domains

Attachments (0)

Change History (18)

comment:1 Changed on 07/29/2014 at 03:56:31 PM by greiner

  • Cc trev added

comment:2 Changed on 09/09/2014 at 09:40:23 AM by greiner

  • Owner set to greiner

comment:3 Changed on 10/23/2014 at 02:12:39 PM by greiner

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

comment:4 Changed on 12/21/2017 at 11:25:42 AM by fhd

  • Cc trev removed

comment:5 Changed on 01/29/2018 at 03:43:02 PM by greiner

  • Description modified (diff)
  • Tester set to Unknown

Added hints for testers section.

comment:6 Changed on 03/24/2019 at 05:41:19 AM by umairaslam


Last edited on 10/08/2019 at 05:25:27 PM by kzar

comment:7 Changed on 04/02/2019 at 02:39:36 PM by greiner

  • Blocked By 7432 added

comment:8 Changed on 04/08/2019 at 10:21:19 AM by Antoniacummins


Last edited on 10/08/2019 at 05:25:31 PM by kzar

comment:9 Changed on 05/06/2019 at 12:08:51 PM by mitchellsroka01


Last edited on 10/08/2019 at 05:25:34 PM by kzar

comment:10 Changed on 05/20/2019 at 09:37:52 AM by Arvind098


Last edited on 10/08/2019 at 05:25:03 PM by kzar

comment:11 Changed on 06/15/2019 at 07:34:42 AM by Angus O'Driscoll


Last edited on 10/08/2019 at 05:25:06 PM by kzar

comment:12 Changed on 06/25/2019 at 11:31:18 AM by Archie Radcliffe


Last edited on 10/08/2019 at 05:25:09 PM by kzar

comment:13 Changed on 06/28/2019 at 08:15:15 AM by Archie Radcliffe


Last edited on 10/08/2019 at 05:25:12 PM by kzar

comment:14 Changed on 07/01/2019 at 07:28:12 AM by Ora Kessler


Last edited on 10/08/2019 at 05:25:15 PM by kzar

comment:15 Changed on 07/18/2019 at 05:50:25 AM by Sperry56


Last edited on 10/08/2019 at 05:25:18 PM by kzar

comment:16 Changed on 08/13/2019 at 12:16:55 PM by Kaitlyn


Last edited on 10/08/2019 at 05:25:21 PM by kzar

comment:17 Changed on 08/15/2019 at 04:37:30 PM by steve39


Last edited on 10/08/2019 at 05:25:24 PM by kzar

comment:18 Changed on 08/29/2019 at 05:43:52 PM by sebastian

  • Keywords closed-in-favor-of-gitlab added
  • Resolution set to rejected
  • Status changed from reviewing to closed

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the new issue tracker.

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from greiner.
Note: See TracTickets for help on using tickets.