Opened on 06/04/2015 at 09:29:24 PM
Closed on 06/05/2015 at 02:07:13 PM
#2642 closed change (fixed)
Move notifications code to separate module
| Reported by: | sebastian | Assignee: | sebastian |
|---|---|---|---|
| Priority: | P3 | Milestone: | Adblock-Plus-1.9-for-Chrome-Opera-Safari |
| Module: | Platform | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Platform: | Unknown | |
| Ready: | yes | Confidential: | no |
| Tester: | Verified working: | no | |
| Review URL(s): | |||
Description
Background
Currently, there is a lot of code related to notifications in background.js. However, that code rather belongs into a separate module.
Moreover, there are 3 places that duplicate code like:
var notificationToShow = NotificationStorage.getNextToShow(stringifyURL(url));
if (notificationToShow)
showNotification(notificationToShow);
So while on it, we should move that logic into showNotification().
What to change
Move notifications code into a new module, add documentation for public functions, replace var with let statements, and move logic to get the next notification into showNotication renaming it to showNextNotification(url)
Hints for testers
The changes here shouldn'T have any user-visible effect. But make sure that the notification mechanism works still as it did before. See 2505#Whattotest for instructions to test the mechanism.
Attachments (0)
Change History (2)
comment:1 Changed on 06/04/2015 at 09:38:44 PM by sebastian
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:2 Changed on 06/05/2015 at 02:07:13 PM by sebastian
- Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
- Resolution set to fixed
- Status changed from reviewing to closed
Note: See
TracTickets for help on using
tickets.
https://hg.adblockplus.org/adblockpluschrome/rev/4240ce6e4550