Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

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

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.

Change History (15)

comment:1 Changed 3 years ago by fhd

  • Blocking 2366 added

comment:2 Changed 3 years ago by fhd

  • Keywords 2015q2 added

comment:3 Changed 3 years ago by fhd

  • Description modified (diff)

comment:4 Changed 3 years ago by fhd

  • Description modified (diff)

comment:5 Changed 3 years ago by fhd

  • Blocked By 2386 added

comment:6 Changed 3 years ago by fhd

  • Description modified (diff)

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

  • Owner set to fhd

comment:9 Changed 3 years ago 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 3 years ago by fhd

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

comment:12 Changed 3 years ago 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 3 years ago 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:15 Changed 3 years ago by fhd

Note: See TracTickets for help on using tickets.