Opened on 04/21/2015 at 09:03:39 AM
Closed on 06/18/2015 at 06:19:15 AM
Last modified on 06/18/2015 at 12:54:58 PM
#2368 closed change (fixed)
[Move notification display logic to Core] Update adblockplus dependency to revision feb391127fe4
Reported by: | fhd | Assignee: | fhd |
---|---|---|---|
Priority: | P2 | Milestone: | Adblock-Plus-1.9.1-for-Chrome-Opera-Safari |
Module: | Platform | Keywords: | 2015q2 |
Cc: | Blocked By: | #2420 | |
Blocking: | #2366 | Platform: | Unknown |
Ready: | yes | Confidential: | no |
Tester: | Verified working: | no | |
Review URL(s): |
https://codereview.adblockplus.org/5733084272001024/ |
Description (last modified by fhd)
Background
The relevant changes imported by this are:
#2420 - Move notification show logic to Core
#2419 - Show newly downloaded notifications immediately
#2659 - Reduce initial notification download delay to one minute
#2390 is also being imported, but shouldn't have any effect.
What to change
Update the adblockplus dependency to revision feb391127fe4.
Attachments (0)
Change History (15)
comment:1 Changed on 04/21/2015 at 09:05:48 AM by fhd
- Blocking 2366 added
comment:2 Changed on 04/21/2015 at 09:12:01 AM by fhd
- Keywords 2015q2 added
comment:5 Changed on 04/22/2015 at 09:25:09 AM by fhd
- Blocked By 2386 added
comment:7 Changed on 04/26/2015 at 09:48:20 AM by fhd
- Blocked By 2420 added; 2386 removed
- Description modified (diff)
- Summary changed from [Show notifications after downloading] in Chrome/Opera/Safari to [Move notification display logic to Core] Use a notification show listener in Chrome/Opera/Safari
comment:8 Changed on 06/08/2015 at 10:58:09 AM by fhd
- Owner set to fhd
comment:9 Changed on 06/12/2015 at 07:21:09 AM by fhd
- Description modified (diff)
- Summary changed from [Move notification display logic to Core] Use a notification show listener in Chrome/Opera/Safari to [Move notification display logic to Core] Update adblockplus dependency to revision feb391127fe4
comment:10 Changed on 06/12/2015 at 07:27:44 AM by fhd
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:11 Changed on 06/18/2015 at 06:18:47 AM by fhd
comment:12 Changed on 06/18/2015 at 06:19:15 AM by fhd
- Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
- Resolution set to fixed
- Status changed from reviewing to closed
comment:13 Changed on 06/18/2015 at 12:35:04 PM by sebastian
- Review URL(s) modified (diff)
Sorry, I overlooked two nits while reviewing this change:
- showNotification() is always called with an object. Therefore the check for !notification is redundant now.
- Redundant wrapper function: showNotification can be passed directly to NotificationStorage.addShowListener.
Here comes a follow up patch.
comment:14 Changed on 06/18/2015 at 12:40:47 PM by sebastian
comment:15 Changed on 06/18/2015 at 12:54:58 PM by fhd
I also forgot a semicolon, bah. Pushed: https://hg.adblockplus.org/adblockpluschrome/rev/5c61b6b4ab9a
Pushed: https://hg.adblockplus.org/adblockpluschrome/rev/e628870bc488