webkit desktop notification #1951

jtg-gg · 2014-06-10T05:48:03Z

Implement webkit dekstop notification (with image url and event dispatcher for click, close, display, error to javascript)
currently only implemented for Windows and MAC

the windows version will need some chrome source modification, I've made the pull request here:
https://github.com/rogerwang/chromium.src/pull/12

mschaaf · 2014-06-10T07:31:15Z

src/nw_notification_manager.cc

+#if defined(OS_MACOSX)
+            singleton_ = new NotificationManagerMac();
+#elif defined(OS_WIN)
+			singleton_ = new NotificationManagerWin();


The indentation is not consistent in this.

sorry about that, I use Xcode and Vstudio, apparently Xcode uses space, Vstudio uses tab

Mithgol · 2014-06-10T13:00:49Z

@jtg-gg It looks gorgeous! 👏

hachre · 2014-06-10T18:34:05Z

@jtg-gg Thanks!

dubcanada · 2014-06-10T18:36:11Z

The indentation and spaces/tabs are all over the place, I do believe node-webkit uses google code style so 4 spaces.

dubcanada · 2014-06-10T18:38:14Z

You should make a placeholder for linux.

rogerwang · 2014-06-11T00:28:57Z

That's for the contribution. It's cool. However, Linux support should be added before it can be merged into master. See also https://groups.google.com/d/msg/node-webkit/W261vvrL2jo/61H23GY2PZkJ

jtg-gg · 2014-06-11T02:55:51Z

@rogerwang , I'll fix the indent ( i just wondering, is 2 spaces or 4 ?) and make the place holder for linux
then if you can integrate to the master, then hopefully some one can help with the linux platform ?

rogerwang · 2014-06-11T04:45:11Z

it's 2 spaces in this repo.

A place holder might help passing build but it's not real implementation, which should be done before merging into master. Or we'll break the cross-platform promise.

I can look into the linux support after 0.10 release.

jtg-gg · 2014-06-11T05:06:51Z

ok thx @rogerwang I've commit the 2 spaces indentation fix and the linux placeholder (i haven't compile it as I don't have linux platform yet)

hachre · 2014-06-11T05:43:59Z

Given the fact that "Linux" has no unified desktop notification system it might be necessary to define exactly what you mean when you say Linux support. Ubuntu? Gnome? KDE? All 3? At least one of them? All 800 window managers you can possibly find? Do you require a workaround for WMs that don't have native desktop notifications? What exactly is the scope here?

jtg-gg · 2014-06-11T05:50:45Z

sorry my bad, is it TOOLKIT_GTK for the linux platform ?

hachre · 2014-06-11T05:52:40Z

@jtg-gg: If you mean me, I was asking the project lead for clarification, not you :)

rogerwang · 2014-06-11T07:25:43Z

@hachre we're not trying to define this. We'll follow upstream.

dubcanada · 2014-06-11T12:09:01Z

I'll take a look at Linux tonight/tomorrow.

dufferzafar · 2014-06-13T19:41:29Z

@jtg-gg I am not in a state to build and test this code myself, can you please provide me the binary (that you might already have) for windows?

jtg-gg · 2014-06-17T03:15:05Z

@dufferzafar I've just made a release here https://github.com/jtg-gg/node-webkit/releases/tag/nw-v0.9.2-jtg-gg

dufferzafar · 2014-06-17T08:24:25Z

@jtg-gg Thanks a ton 😄

djcas9 · 2014-06-17T21:19:05Z

@jtg-gg does that include your setBadgeLabel code as well?

jtg-gg · 2014-06-18T02:02:25Z

@mephux Yes it does include the setBadgeLabel code

elronalds · 2014-06-20T11:58:43Z

@jtg-gg Have you thought about adding support for the new Windows 8/8.1 notifications using the Windows.UI.Notifications shown in the top right corner of the screen? That might be useful if the app isn't providing a system tray icon and if targeting the newer OS.

gpetrov · 2014-06-22T10:09:00Z

@rogerwang could you please make sure this is added in nw 0.10.0

it is such an essential part! Even if it is only for OSX and Windows.
As outlined above on Linux there just too many ways of doing this and we can not wait.

Please merge asap!

octalmage · 2014-06-22T12:45:03Z

Currently I use this on Mac, works great!

osascript -e 'display notification "Lorem ipsum dolor sit amet" with title "Title"'

dufferzafar · 2014-06-22T20:53:53Z

@jtg-gg The release you've supplied for windows does not work. This is the error I get when trying to execute nw.exe

Problem signature:
  Problem Event Name:   APPCRASH
  Application Name: nw.exe
  Application Version:  0.0.0.0
  Application Timestamp:    539ec0b4
  Fault Module Name:    nw.exe
  Fault Module Version: 0.0.0.0
  Fault Module Timestamp:   539ec0b4
  Exception Code:   80000003
  Exception Offset: 000a7610
  OS Version:   6.1.7601.2.1.0.768.2
  Locale ID:    1033
  Additional Information 1: 0a9e
  Additional Information 2: 0a9e372d3b4ad19135b953a78882e789
  Additional Information 3: 0a9e
  Additional Information 4: 0a9e372d3b4ad19135b953a78882e789

Read our privacy statement online:
  http://go.microsoft.com/fwlink/?linkid=104288&clcid=0x0409

If the online privacy statement is not available, please read our privacy statement offline:
  C:\Windows\system32\en-US\erofflps.txt

djcas9 · 2014-06-22T20:56:01Z

@octalmage not sure how that would be a replacement for native notification support? How do you handle on click events?

octalmage · 2014-06-23T10:43:29Z

@mephux Personally I only need to alert the user so for me it's a good work around until native notification support is implemented.

jamesmortensen · 2014-06-23T16:12:05Z

@dufferzafar Are you sure you copied the nwsnapshot.exe and the *.dll's from an existing 0.9.2 build? @jtg-gg didn't ship all the files for Windows so you have to copy them yourself.

Hope this helps!

dufferzafar · 2014-06-23T16:40:27Z

@jamesmortensen Thanks. That worked :)

sishen · 2014-07-22T02:05:43Z

@jtg-gg Thank you very much for the awesome work. I have a problem which I want to confirm whether belongs to Chromuium or node-webkit. So I'll try with the Chromium 35.0.1916.113. Does the notifications addon compatible with the v0.10.0-rc2? Is that possible to make one release, if yes it'll be very helpful, thank you!

rogerwang · 2014-07-22T06:26:02Z

@sishen I believe the patches should be updated to build with v0.10.0. @jtg-gg please update. Thanks.

jtg-gg · 2014-07-22T07:57:59Z

Hi @sishen , @rogerwang will try to update, might need lots of works on my side
meanwhile I've been developing the linux ubuntu notification, using libnotify, here is the screen shot.
Ubuntu notification does not support click event, so I can't sent that event back to javascript

sishen · 2014-07-22T08:06:47Z

@jtg-gg That looks great! Thank you for all your efforts.

rogerwang · 2014-07-23T02:22:29Z

@jtg-gg I found that upstream uses Balloon notification on Linux, similar with what you are using on windows, could your impl be based on that? see https://github.com/rogerwang/chromium.src/tree/nw/chrome/browser/ui/gtk/status_icons

Squashed commits: [b3c450f] [Notification] handle empty body on windows (+1 squashed commits) Squashed commits: [ca3f634] [Notification] fix compiler error. missing base:: (+1 squashed commits) Squashed commits: [af8b22b] [Notification]fix compile error on linux (+1 squashed commit) Squashed commits: [32bc400] compile error fix (+1 squashed commit) Squashed commits: [2955c46] [linux] notification implementation (+1 squashed commit) Squashed commits: [edb7e83] [WIN] make the StatusTray a singleton, check for the StatusIcon if it has been created before, handle the destruction of StatusIcon and StatusObserver properly (+1 squashed commit) Squashed commits: [3a1f53b] [Linux] add placeholder for the desktop notification (+7 squashed commits) Squashed commits: [f784e4a] indentation and formatting fix [c5ef03d] fix the indentation [d1662e7] [MAC] add version check for the notification image, since it only supported by Mavericks [18b2d38] [WIN] fix notification manager compile warnings (missing OVERRIDE) [00177bb] [MAC] implement loading / showing image from url for desktop notification [e8b8cce] fix compile warning for undefined constructor [a3d73b2] Move notification image download callback to the parent class, preparation for MAC notification custom icon implementation (+1 squashed commit) Squashed commits: [ee010e2] [WIN] implement the icon image display from URL (+8 squashed commits) Squashed commits: [4844475] [MAC] implement desktop notification, display event dispatcher [4c5f445] Remove unnecessary comments [af4bd7e] Implement notification events dispatcher (close, display, error) [d6662bc] [WIN] implement windows balloon tray notification, with application icon as the tray icon [19c8965] [WIN] AddDesktopNotification, CancelDesktopNotification function prototype adjustment with NotificationManager [29816f5] [WIN] implement windows notification [7191831] [MACOSX] implement desktop notification event handler (send the click back to java script) implement "CancelDesktopNotification" [c5b359d] add notification manager (nw_notification_manager) for desktop notification implement MacOS notification manager

jtg-gg · 2014-07-29T05:27:18Z

@rogerwang I've updated the pull request to build 0.10, now it includes linux implementation. However I've looked for the possibility to do linux balloon notification, however it needs lots of works, the tray implementation is broken for ubuntu, needs lots of customisation. hence I keep the libnotify implementation

this pull request will need the chrome source code modification, here https://github.com/rogerwang/chromium.src/pull/12

webkit desktop notification

island205 · 2014-08-06T01:51:43Z

nice work!

1inus · 2017-06-17T05:29:17Z

太棒了

jtg-gg mentioned this pull request Jun 10, 2014

Implement desktop notifications [$100 awarded] #27

Closed

mschaaf reviewed Jun 10, 2014
View reviewed changes

jtg-gg closed this Jul 29, 2014

jtg-gg deleted the notification branch July 29, 2014 05:21

jtg-gg reopened this Jul 29, 2014

rogerwang added a commit that referenced this pull request Jul 29, 2014

Merge pull request #1951 from jtg-gg/notification

39e0731

webkit desktop notification

rogerwang merged commit 39e0731 into nwjs:master Jul 29, 2014

semmel mentioned this pull request Jul 31, 2014

Creating multiple tray icons in Windows 7+ is broken on nw 0.8.0 - 0.10.1 #1402

Closed

ready4god2513 mentioned this pull request Aug 9, 2014

Document the new Notifications API #2184

Closed

Constellation mentioned this pull request Aug 13, 2014

version up node-webkit djcas9/komanda#191

Closed

octalmage mentioned this pull request Aug 25, 2014

Reminders? octalmage/Marknote#5

Open

webkit desktop notification #1951

webkit desktop notification #1951

Conversation

jtg-gg commented Jun 10, 2014

mschaaf Jun 10, 2014

Choose a reason for hiding this comment

jtg-gg Jun 10, 2014

Choose a reason for hiding this comment

Mithgol commented Jun 10, 2014

hachre commented Jun 10, 2014

dubcanada commented Jun 10, 2014

dubcanada commented Jun 10, 2014

rogerwang commented Jun 11, 2014

jtg-gg commented Jun 11, 2014

rogerwang commented Jun 11, 2014

jtg-gg commented Jun 11, 2014

hachre commented Jun 11, 2014

jtg-gg commented Jun 11, 2014

hachre commented Jun 11, 2014

rogerwang commented Jun 11, 2014

dubcanada commented Jun 11, 2014

dufferzafar commented Jun 13, 2014

jtg-gg commented Jun 17, 2014

dufferzafar commented Jun 17, 2014

djcas9 commented Jun 17, 2014

jtg-gg commented Jun 18, 2014

elronalds commented Jun 20, 2014

gpetrov commented Jun 22, 2014

octalmage commented Jun 22, 2014

dufferzafar commented Jun 22, 2014

djcas9 commented Jun 22, 2014

octalmage commented Jun 23, 2014

jamesmortensen commented Jun 23, 2014

dufferzafar commented Jun 23, 2014

sishen commented Jul 22, 2014

rogerwang commented Jul 22, 2014

jtg-gg commented Jul 22, 2014

sishen commented Jul 22, 2014

rogerwang commented Jul 23, 2014

jtg-gg commented Jul 29, 2014

island205 commented Aug 6, 2014

1inus commented Jun 17, 2017