Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 6098518317989888: Fix: Notification popup is broken (Closed)

Created:
Feb. 12, 2014, 9:39 a.m. by saroyanm
Modified:
March 18, 2014, 5:17 p.m.
Visibility:
Public.

Description

Notifications api: I've noticed that chrome has a notification API (v28 and above) -> https://developer.chrome.com/extensions/notifications.html, the API use template to generate notifications and also have ability to use buttons in the notification within the template, but the button will appear below notification and we again can't use HTML template. There is a video describing the notification API -> https://developers.google.com/live/shows/83992232-1001 Anyway the current solution is still work Okey until we would like to have buttons within the notification or image beside the icon, so not sure whether we should stick to this solution and continue with other issues, or should the patch be changed to use Notifications API instead. I haven't removed the old createHTMLNotification method for old versions of chrome, the reason was I was not sure whether ABP still need to support createHTMLNotification for old versions of browser. So in case we need to get rid of it just let me know. Test notes: adblockplus.js line 42: you can change "notificationurl" to use notification.json hosted locally within your local server, or update notificationdata object. adblockplus.js line 3611: Also make sense to change INITIAL_DELAY to get notification.json with less delay in case you would like to test functionality. Additional notes: Also noticed that if we are providing several critical notifications, only the first one always are shown, so in case that behavior should be changed some additional discuss needed here.

Patch Set 1 #

Patch Set 2 : chrome.notification added #

Total comments: 30

Patch Set 3 : Changes after Thomas notes #

Total comments: 18

Patch Set 4 : Linux issue and minor changes #

Total comments: 6

Patch Set 5 : Already there #

Total comments: 5

Patch Set 6 : moved code from /extcommon to ext/background #

Total comments: 16

Patch Set 7 : Felix review changes #

Total comments: 9

Patch Set 8 : Minore changes #

Total comments: 1

Patch Set 9 : Notification without button default text #

Total comments: 6

Patch Set 10 : Notification info msg correction. #

Patch Set 11 : Some fixes in code #

Total comments: 8

Patch Set 12 : Popup notification fix #

Total comments: 5

Patch Set 13 : Change back activeNotification unset #

Total comments: 25
Unified diffs Side-by-side diffs Delta from patch set Stats (+97 lines, -8 lines) Patch
M _locales/en_US/messages.json View 1 2 3 4 5 6 7 8 9 1 chunk +6 lines, -0 lines 0 comments Download
M background.js View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +85 lines, -7 lines 2 comments Download
M chrome/ext/background.js View 1 2 3 4 5 6 1 chunk +3 lines, -0 lines 8 comments Download
M metadata.chrome View 1 1 chunk +1 line, -1 line 0 comments Download
M notification.html View 1 2 3 1 chunk +2 lines, -0 lines 15 comments Download

Messages

Total messages: 81
Thomas Greiner
We use notifications for a different variety of call-for-actions. Displaying text is not enough for ...
Feb. 12, 2014, 3:34 p.m. (2014-02-12 15:34:05 UTC) #1
saroyanm
> If Web Notifications don't allow listening to clicks we need to find a different ...
Feb. 14, 2014, 4:33 p.m. (2014-02-14 16:33:48 UTC) #2
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/5741031244955648/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5741031244955648/background.js#newcode295 background.js:295: function notificationWindowClick() { Nit: Bracket on next line http://codereview.adblockplus.org/6098518317989888/diff/5741031244955648/background.js#newcode300 ...
Feb. 17, 2014, 10:02 a.m. (2014-02-17 10:02:40 UTC) #3
saroyanm
Thanks for the reviewing and notes Thomas, that's really helpful I fill myself getting more ...
Feb. 18, 2014, 10:25 a.m. (2014-02-18 10:25:08 UTC) #4
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/5741031244955648/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5741031244955648/background.js#newcode329 background.js:329: if ("notifications" in chrome) On 2014/02/18 10:25:08, saroyanm wrote: ...
Feb. 18, 2014, 2:26 p.m. (2014-02-18 14:26:17 UTC) #5
saroyanm
Hope this will good for us to commit. http://codereview.adblockplus.org/6098518317989888/diff/5733935958982656/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5733935958982656/background.js#newcode322 background.js:322: function ...
Feb. 18, 2014, 4:20 p.m. (2014-02-18 16:20:28 UTC) #6
Thomas Greiner
Almost there. http://codereview.adblockplus.org/6098518317989888/diff/5738600293466112/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5738600293466112/background.js#newcode338 background.js:338: var message = texts.message ? texts.message.replace(new RegExp("<(a|\/a)>", ...
Feb. 18, 2014, 4:44 p.m. (2014-02-18 16:44:25 UTC) #7
saroyanm
Fingers crossed :) http://codereview.adblockplus.org/6098518317989888/diff/5738600293466112/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5738600293466112/background.js#newcode338 background.js:338: var message = texts.message ? texts.message.replace(new ...
Feb. 18, 2014, 5:53 p.m. (2014-02-18 17:53:48 UTC) #8
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/6563422992859136/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/6563422992859136/background.js#newcode350 background.js:350: var message = texts.message ? texts.message : ""; To ...
Feb. 19, 2014, 9:25 a.m. (2014-02-19 09:25:56 UTC) #9
saroyanm
Changes according last review comments. Hope the change for ext/background is the correct, can you ...
Feb. 19, 2014, 1:14 p.m. (2014-02-19 13:14:42 UTC) #10
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/6563422992859136/chrome/ext/common.js File chrome/ext/common.js (right): http://codereview.adblockplus.org/6098518317989888/diff/6563422992859136/chrome/ext/common.js#newcode116 chrome/ext/common.js:116: // Chrome on Linux does not fully support chrome.notifications ...
Feb. 20, 2014, 3:58 p.m. (2014-02-20 15:58:36 UTC) #11
saroyanm
On 2014/02/20 15:58:36, Thomas Greiner wrote: > http://codereview.adblockplus.org/6098518317989888/diff/6563422992859136/chrome/ext/common.js > File chrome/ext/common.js (right): > > http://codereview.adblockplus.org/6098518317989888/diff/6563422992859136/chrome/ext/common.js#newcode116 ...
Feb. 20, 2014, 4:39 p.m. (2014-02-20 16:39:22 UTC) #12
Thomas Greiner
LGTM
Feb. 20, 2014, 5:11 p.m. (2014-02-20 17:11:01 UTC) #13
Felix Dahlke
Here's some more from me then :) http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js#newcode337 background.js:337: var title ...
Feb. 21, 2014, 9:48 a.m. (2014-02-21 09:48:49 UTC) #14
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js#newcode340 background.js:340: if ("notifications" in ext) On 2014/02/21 09:48:49, Felix H. ...
Feb. 21, 2014, 9:53 a.m. (2014-02-21 09:53:42 UTC) #15
Felix Dahlke
http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js#newcode340 background.js:340: if ("notifications" in ext) On 2014/02/21 09:53:43, Thomas Greiner ...
Feb. 21, 2014, 9:56 a.m. (2014-02-21 09:56:22 UTC) #16
Felix Dahlke
http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js#newcode340 background.js:340: if ("notifications" in ext) On 2014/02/21 09:56:22, Felix H. ...
Feb. 21, 2014, 10:04 a.m. (2014-02-21 10:04:00 UTC) #17
saroyanm
Hope I understand guys your point regarding the notifications correctly. http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js#newcode337 ...
Feb. 21, 2014, 1:39 p.m. (2014-02-21 13:39:22 UTC) #18
Felix Dahlke
http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js#newcode352 background.js:352: opt.buttons.push({title: RegExp.$1}); On 2014/02/21 13:39:22, saroyanm wrote: > On ...
Feb. 21, 2014, 1:48 p.m. (2014-02-21 13:48:20 UTC) #19
Felix Dahlke
Almost there :) http://codereview.adblockplus.org/6098518317989888/diff/5723151296102400/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5723151296102400/background.js#newcode351 background.js:351: while (match = regex.exec(plainMessage)) As I ...
Feb. 21, 2014, 1:53 p.m. (2014-02-21 13:53:40 UTC) #20
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js#newcode340 background.js:340: if ("notifications" in ext) On 2014/02/21 13:39:22, saroyanm wrote: ...
Feb. 21, 2014, 1:59 p.m. (2014-02-21 13:59:55 UTC) #21
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/5723151296102400/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5723151296102400/background.js#newcode351 background.js:351: while (match = regex.exec(plainMessage)) Avoid making "match" a global ...
Feb. 21, 2014, 2:35 p.m. (2014-02-21 14:35:53 UTC) #22
Felix Dahlke
http://codereview.adblockplus.org/6098518317989888/diff/5723151296102400/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5723151296102400/background.js#newcode351 background.js:351: while (match = regex.exec(plainMessage)) On 2014/02/21 14:35:53, Thomas Greiner ...
Feb. 21, 2014, 3:01 p.m. (2014-02-21 15:01:48 UTC) #23
saroyanm
Fingers crossed. http://codereview.adblockplus.org/6098518317989888/diff/5723151296102400/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5723151296102400/background.js#newcode351 background.js:351: while (match = regex.exec(plainMessage)) On 2014/02/21 13:53:40, ...
Feb. 21, 2014, 3:03 p.m. (2014-02-21 15:03:25 UTC) #24
saroyanm
http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5728116278296576/background.js#newcode340 background.js:340: if ("notifications" in ext) On 2014/02/21 13:59:55, Thomas Greiner ...
Feb. 21, 2014, 3:11 p.m. (2014-02-21 15:11:51 UTC) #25
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/5723151296102400/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5723151296102400/background.js#newcode351 background.js:351: while (match = regex.exec(plainMessage)) On 2014/02/21 15:03:25, saroyanm wrote: ...
Feb. 21, 2014, 3:18 p.m. (2014-02-21 15:18:42 UTC) #26
saroyanm
http://codereview.adblockplus.org/6098518317989888/diff/5723151296102400/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5723151296102400/background.js#newcode351 background.js:351: while (match = regex.exec(plainMessage)) On 2014/02/21 15:18:43, Thomas Greiner ...
Feb. 21, 2014, 3:29 p.m. (2014-02-21 15:29:00 UTC) #27
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/5659645909663744/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5659645909663744/background.js#newcode362 background.js:362: var notification = webkitNotifications.createNotification(iconUrl, title, message); Shouldn't this notification's ...
Feb. 21, 2014, 3:54 p.m. (2014-02-21 15:54:51 UTC) #28
saroyanm
On 2014/02/21 15:54:51, Thomas Greiner wrote: > http://codereview.adblockplus.org/6098518317989888/diff/5659645909663744/background.js > File background.js (right): > > http://codereview.adblockplus.org/6098518317989888/diff/5659645909663744/background.js#newcode362 ...
Feb. 21, 2014, 4:22 p.m. (2014-02-21 16:22:56 UTC) #29
saroyanm
On 2014/02/21 16:22:56, saroyanm wrote: > On 2014/02/21 15:54:51, Thomas Greiner wrote: > > > ...
Feb. 21, 2014, 4:44 p.m. (2014-02-21 16:44:40 UTC) #30
Felix Dahlke
Aaaaalmost there :) http://codereview.adblockplus.org/6098518317989888/diff/5703128158568448/_locales/en_US/messages.json File _locales/en_US/messages.json (right): http://codereview.adblockplus.org/6098518317989888/diff/5703128158568448/_locales/en_US/messages.json#newcode103 _locales/en_US/messages.json:103: "message": "Click 'OK' to open the ...
Feb. 21, 2014, 7:17 p.m. (2014-02-21 19:17:37 UTC) #31
saroyanm
Changed the messages for notification. http://codereview.adblockplus.org/6098518317989888/diff/5703128158568448/_locales/en_US/messages.json File _locales/en_US/messages.json (right): http://codereview.adblockplus.org/6098518317989888/diff/5703128158568448/_locales/en_US/messages.json#newcode103 _locales/en_US/messages.json:103: "message": "Click 'OK' to ...
Feb. 22, 2014, 10:22 a.m. (2014-02-22 10:22:15 UTC) #32
saroyanm
Hi guys, sorry for this last update. Actually I've noticed that "prepareNotificationIconAndPopup" used to be ...
Feb. 22, 2014, 5:03 p.m. (2014-02-22 17:03:01 UTC) #33
saroyanm
To be more concrete regarding not showing notification in popup window after closing desktop notificaiton ...
Feb. 22, 2014, 5:11 p.m. (2014-02-22 17:11:26 UTC) #34
Thomas Greiner
On 2014/02/22 17:03:01, saroyanm wrote: > Hi guys, > sorry for this last update. > ...
Feb. 24, 2014, 2:35 p.m. (2014-02-24 14:35:59 UTC) #35
saroyanm
http://codereview.adblockplus.org/6098518317989888/diff/5890822524370944/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5890822524370944/background.js#newcode292 background.js:292: iconAnimation.update(activeNotification.severity); Felix, what you think do we need this ...
Feb. 26, 2014, 3:52 p.m. (2014-02-26 15:52:18 UTC) #36
saroyanm
> On 2014/02/22 17:11:26, saroyanm wrote: > > To be more concrete regarding not showing ...
Feb. 26, 2014, 4:01 p.m. (2014-02-26 16:01:01 UTC) #37
Thomas Greiner
> Actually I'm not sure regarding that we are assigning activeNotification in > showNotification method, ...
Feb. 26, 2014, 4:25 p.m. (2014-02-26 16:25:46 UTC) #38
saroyanm
On 2014/02/26 16:25:46, Thomas Greiner wrote: > > Actually I'm not sure regarding that we ...
Feb. 26, 2014, 4:33 p.m. (2014-02-26 16:33:17 UTC) #39
Felix Dahlke
http://codereview.adblockplus.org/6098518317989888/diff/5890822524370944/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5890822524370944/background.js#newcode292 background.js:292: iconAnimation.update(activeNotification.severity); On 2014/02/26 15:52:18, saroyanm wrote: > Felix, what ...
Feb. 26, 2014, 5:22 p.m. (2014-02-26 17:22:58 UTC) #40
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/5758531089203200/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5758531089203200/background.js#newcode290 background.js:290: On 2014/02/26 17:22:58, Felix H. Dahlke wrote: > Why ...
Feb. 26, 2014, 5:37 p.m. (2014-02-26 17:37:35 UTC) #41
Felix Dahlke
http://codereview.adblockplus.org/6098518317989888/diff/5758531089203200/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5758531089203200/background.js#newcode290 background.js:290: On 2014/02/26 17:37:35, Thomas Greiner wrote: > On 2014/02/26 ...
Feb. 26, 2014, 5:47 p.m. (2014-02-26 17:47:10 UTC) #42
Thomas Greiner
> Anyway, calling that right before we show a notification in all cases below > ...
Feb. 26, 2014, 5:52 p.m. (2014-02-26 17:52:11 UTC) #43
saroyanm
http://codereview.adblockplus.org/6098518317989888/diff/5890822524370944/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5890822524370944/background.js#newcode379 background.js:379: prepareNotificationIconAndPopup(); On 2014/02/26 17:22:58, Felix H. Dahlke wrote: > ...
Feb. 26, 2014, 6 p.m. (2014-02-26 18:00:29 UTC) #44
Felix Dahlke
http://codereview.adblockplus.org/6098518317989888/diff/5890822524370944/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5890822524370944/background.js#newcode379 background.js:379: prepareNotificationIconAndPopup(); On 2014/02/26 18:00:29, saroyanm wrote: > On 2014/02/26 ...
Feb. 26, 2014, 6:02 p.m. (2014-02-26 18:02:53 UTC) #45
saroyanm
http://codereview.adblockplus.org/6098518317989888/diff/5890822524370944/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5890822524370944/background.js#newcode292 background.js:292: iconAnimation.update(activeNotification.severity); On 2014/02/26 17:22:58, Felix H. Dahlke wrote: > ...
Feb. 26, 2014, 6:15 p.m. (2014-02-26 18:15:34 UTC) #46
Felix Dahlke
On 2014/02/26 18:15:34, saroyanm wrote: > http://codereview.adblockplus.org/6098518317989888/diff/5890822524370944/background.js > File background.js (right): > > http://codereview.adblockplus.org/6098518317989888/diff/5890822524370944/background.js#newcode292 > ...
Feb. 26, 2014, 6:19 p.m. (2014-02-26 18:19:10 UTC) #47
Felix Dahlke
Feb. 26, 2014, 6:19 p.m. (2014-02-26 18:19:32 UTC) #48
saroyanm
http://codereview.adblockplus.org/6098518317989888/diff/5758531089203200/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5758531089203200/background.js#newcode290 background.js:290: On 2014/02/26 18:15:34, saroyanm wrote: > On 2014/02/26 17:47:10, ...
Feb. 26, 2014, 6:25 p.m. (2014-02-26 18:25:08 UTC) #49
Felix Dahlke
On 2014/02/26 18:25:08, saroyanm wrote: > From My point of view, the logic should change: ...
Feb. 26, 2014, 6:30 p.m. (2014-02-26 18:30:03 UTC) #50
saroyanm
On 2014/02/26 18:30:03, Felix H. Dahlke wrote: > On 2014/02/26 18:25:08, saroyanm wrote: > > ...
Feb. 26, 2014, 6:40 p.m. (2014-02-26 18:40:04 UTC) #51
Felix Dahlke
On 2014/02/26 18:40:04, saroyanm wrote: > After closing Desktop notification the prepareNotificationIconAndPopup method is > ...
Feb. 26, 2014, 6:48 p.m. (2014-02-26 18:48:40 UTC) #52
saroyanm
> activeNotification.onClicked is not called when the notification is closed, but > when it's shown ...
Feb. 26, 2014, 7:02 p.m. (2014-02-26 19:02:36 UTC) #53
saroyanm
> Can you check if it works without the current revision, i.e. without any of ...
Feb. 26, 2014, 7:07 p.m. (2014-02-26 19:07:58 UTC) #54
saroyanm
Guys while the implementation of notification.js is not really functional: * iconAnimation is not working ...
Feb. 27, 2014, 9:30 a.m. (2014-02-27 09:30:39 UTC) #55
Thomas Greiner
If Felix agrees we can do the suggested change in a separate review. Anyway, LGTM ...
Feb. 27, 2014, 10:07 a.m. (2014-02-27 10:07:32 UTC) #56
Felix Dahlke
On 2014/02/27 09:30:39, saroyanm wrote: > Guys while the implementation of notification.js is not really ...
Feb. 27, 2014, 2:08 p.m. (2014-02-27 14:08:05 UTC) #57
Felix Dahlke
LGTM with mine and greiner's nit addressed! http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/background.js File background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/background.js#newcode292 background.js:292: iconAnimation.update(activeNotification.severity); Nit: ...
Feb. 27, 2014, 2:10 p.m. (2014-02-27 14:10:38 UTC) #58
Felix Dahlke
Noticed one thing I didn't notice before: http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html File notification.html (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html#newcode30 notification.html:30: <script src="ext/background.js"></script> ...
Feb. 27, 2014, 2:14 p.m. (2014-02-27 14:14:43 UTC) #59
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html File notification.html (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html#newcode30 notification.html:30: <script src="ext/background.js"></script> On 2014/02/27 14:14:43, Felix H. Dahlke wrote: ...
Feb. 27, 2014, 3:10 p.m. (2014-02-27 15:10:37 UTC) #60
Sebastian Noack
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js#newcode584 chrome/ext/background.js:584: ext.browserNotifications = chrome.notifications; This doesn't belong into the abstraction ...
Feb. 28, 2014, 9:56 a.m. (2014-02-28 09:56:03 UTC) #61
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js#newcode584 chrome/ext/background.js:584: ext.browserNotifications = chrome.notifications; On 2014/02/28 09:56:03, Sebastian Noack wrote: ...
Feb. 28, 2014, 10:10 a.m. (2014-02-28 10:10:55 UTC) #62
Sebastian Noack
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js#newcode584 chrome/ext/background.js:584: ext.browserNotifications = chrome.notifications; On 2014/02/28 10:10:55, Thomas Greiner wrote: ...
Feb. 28, 2014, 10:19 a.m. (2014-02-28 10:19:44 UTC) #63
saroyanm
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html File notification.html (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html#newcode30 notification.html:30: <script src="ext/background.js"></script> On 2014/02/28 09:56:03, Sebastian Noack wrote: > ...
Feb. 28, 2014, 10:56 a.m. (2014-02-28 10:56:12 UTC) #64
saroyanm
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js#newcode584 chrome/ext/background.js:584: ext.browserNotifications = chrome.notifications; On 2014/02/28 10:19:45, Sebastian Noack wrote: ...
Feb. 28, 2014, 11:18 a.m. (2014-02-28 11:18:43 UTC) #65
saroyanm
On 2014/02/28 11:18:43, saroyanm wrote: > http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js > File chrome/ext/background.js (right): > > http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js#newcode584 > ...
Feb. 28, 2014, 11:37 a.m. (2014-02-28 11:37:06 UTC) #66
Sebastian Noack
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js#newcode584 chrome/ext/background.js:584: ext.browserNotifications = chrome.notifications; On 2014/02/28 11:18:43, saroyanm wrote: > ...
Feb. 28, 2014, 12:35 p.m. (2014-02-28 12:35:02 UTC) #67
saroyanm
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js#newcode584 chrome/ext/background.js:584: ext.browserNotifications = chrome.notifications; On 2014/02/28 12:35:03, Sebastian Noack wrote: ...
Feb. 28, 2014, 1:47 p.m. (2014-02-28 13:47:04 UTC) #68
Sebastian Noack
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html File notification.html (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html#newcode30 notification.html:30: <script src="ext/background.js"></script> On 2014/02/28 13:47:04, saroyanm wrote: > On ...
Feb. 28, 2014, 2:23 p.m. (2014-02-28 14:23:49 UTC) #69
saroyanm
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html File notification.html (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html#newcode30 notification.html:30: <script src="ext/background.js"></script> On 2014/02/28 14:23:49, Sebastian Noack wrote: > ...
Feb. 28, 2014, 2:34 p.m. (2014-02-28 14:34:25 UTC) #70
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js#newcode584 chrome/ext/background.js:584: ext.browserNotifications = chrome.notifications; On 2014/02/28 13:47:04, saroyanm wrote: > ...
Feb. 28, 2014, 4:04 p.m. (2014-02-28 16:04:14 UTC) #71
Sebastian Noack
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/chrome/ext/background.js#newcode584 chrome/ext/background.js:584: ext.browserNotifications = chrome.notifications; On 2014/02/28 16:04:14, Thomas Greiner wrote: ...
Feb. 28, 2014, 4:59 p.m. (2014-02-28 16:59:22 UTC) #72
Sebastian Noack
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html File notification.html (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html#newcode30 notification.html:30: <script src="ext/background.js"></script> On 2014/02/28 16:59:22, Sebastian Noack wrote: > ...
Feb. 28, 2014, 5:01 p.m. (2014-02-28 17:01:00 UTC) #73
saroyanm
@Thomas can you please reply to my last comment, Thanks in advance. http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html File notification.html ...
March 3, 2014, 7:08 a.m. (2014-03-03 07:08:51 UTC) #74
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html File notification.html (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html#newcode30 notification.html:30: <script src="ext/background.js"></script> On 2014/03/03 07:08:51, saroyanm wrote: > On ...
March 4, 2014, 9:28 a.m. (2014-03-04 09:28:14 UTC) #75
Sebastian Noack
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html File notification.html (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html#newcode30 notification.html:30: <script src="ext/background.js"></script> On 2014/03/03 07:08:51, saroyanm wrote: > Sebastian ...
March 5, 2014, 10:45 a.m. (2014-03-05 10:45:48 UTC) #76
Thomas Greiner
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html File notification.html (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html#newcode30 notification.html:30: <script src="ext/background.js"></script> > > to use createNotification for old ...
March 5, 2014, 11:30 a.m. (2014-03-05 11:30:51 UTC) #77
saroyanm
http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html File notification.html (right): http://codereview.adblockplus.org/6098518317989888/diff/5651874166341632/notification.html#newcode30 notification.html:30: <script src="ext/background.js"></script> > > to use createNotification for old ...
March 5, 2014, 11:33 a.m. (2014-03-05 11:33:25 UTC) #78
Felix Dahlke
Wow, 129 comments, and I'm not really sure what the bottom line of this is ...
March 5, 2014, 9:09 p.m. (2014-03-05 21:09:20 UTC) #79
Sebastian Noack
On 2014/03/05 21:09:20, Felix H. Dahlke wrote: > Wow, 129 comments, and I'm not really ...
March 5, 2014, 9:14 p.m. (2014-03-05 21:14:44 UTC) #80
Wladimir Palant
March 18, 2014, 5:17 p.m. (2014-03-18 17:17:31 UTC) #81
Filed https://issues.adblockplus.org/ticket/172 as follow-up to this.

Powered by Google App Engine
This is Rietveld