New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement desktop notifications [$100 awarded] #27

Closed
zcbenz opened this Issue Aug 26, 2012 · 111 comments

Comments

Projects
None yet
@zcbenz
Contributor

zcbenz commented Aug 26, 2012

webkitNotifications can be implemented by overriding ShowDesktopNotification and CancelDesktopNotification in content browser client, chrome has already implemented them in chrome/browser/notifications, code can be reused there.

The $100 bounty on this issue has been claimed at Bountysource.

@zcbenz zcbenz referenced this issue Nov 23, 2012

Closed

Notification API #212

@Mithgol

This comment has been minimized.

Show comment
Hide comment
@Mithgol

Mithgol Dec 22, 2012

Contributor

Hyperlinks for the future docs on this feature:

Contributor

Mithgol commented Dec 22, 2012

Hyperlinks for the future docs on this feature:

@Mithgol

This comment has been minimized.

Show comment
Hide comment
@Mithgol

Mithgol Dec 26, 2012

Contributor

And somebody should tell that user to watch this issue.

Contributor

Mithgol commented Dec 26, 2012

And somebody should tell that user to watch this issue.

@AndryBray

This comment has been minimized.

Show comment
Hide comment
@AndryBray

AndryBray Jun 7, 2013

Hi! Any news about this feature? Hope yes =)

AndryBray commented Jun 7, 2013

Hi! Any news about this feature? Hope yes =)

@ChaitanyaPramod

This comment has been minimized.

Show comment
Hide comment
@ChaitanyaPramod

ChaitanyaPramod Jun 11, 2013

Desktop Notifications are very critical for any App to give a completely native experience. And for a chat app (which I'm trying to port from a chrome-extension), notifications are essential.
I wonder if anyone has tried the implementation suggested by @zcbenz in the description? If it looks simple enough, I might try it.

ChaitanyaPramod commented Jun 11, 2013

Desktop Notifications are very critical for any App to give a completely native experience. And for a chat app (which I'm trying to port from a chrome-extension), notifications are essential.
I wonder if anyone has tried the implementation suggested by @zcbenz in the description? If it looks simple enough, I might try it.

@damianb

This comment has been minimized.

Show comment
Hide comment
@damianb

damianb commented Jun 14, 2013

+1

@AndryBray

This comment has been minimized.

Show comment
Hide comment
@AndryBray

AndryBray commented Jun 21, 2013

+1

@Anonyfox

This comment has been minimized.

Show comment
Hide comment
@Anonyfox

Anonyfox commented Jun 21, 2013

+1

@luxueyan2008

This comment has been minimized.

Show comment
Hide comment
@luxueyan2008

luxueyan2008 commented Jun 22, 2013

me too

@YurySolovyov

This comment has been minimized.

Show comment
Hide comment
@YurySolovyov

YurySolovyov commented Jun 23, 2013

im not sure, but chrome 28+ should have notifications http://blog.chromium.org/2013/05/rich-notifications-in-chrome.html

@damianb

This comment has been minimized.

Show comment
Hide comment
@damianb

damianb Jun 23, 2013

prior versions of chrome had it buried too under chrome://flags

damianb commented Jun 23, 2013

prior versions of chrome had it buried too under chrome://flags

@Mithgol

This comment has been minimized.

Show comment
Hide comment
@Mithgol

Mithgol Jun 24, 2013

Contributor

I wonder if it does really mean that they appear then in Chromium/Blink code and then in node-webkit.

Contributor

Mithgol commented Jun 24, 2013

I wonder if it does really mean that they appear then in Chromium/Blink code and then in node-webkit.

@AndryBray

This comment has been minimized.

Show comment
Hide comment
@AndryBray

AndryBray Jun 28, 2013

Please, could we know the place / priority of this feature in the roadmap?
Thank you

AndryBray commented Jun 28, 2013

Please, could we know the place / priority of this feature in the roadmap?
Thank you

@Mithgol

This comment has been minimized.

Show comment
Hide comment
@Mithgol

Mithgol Jun 28, 2013

Contributor

@Dream707

im not sure, but chrome 28+ should have notifications http://blog.chromium.org/2013/05/rich-notifications-in-chrome.html

That's rich notifications for Chrome packaged apps and extensions. A different thing.

Here we are talking about HTML5 Notifications API.

The real issue here is that typeof Notification === 'function' already (in node-webkit 0.6.1) and that you can call new Notification('some test'), but nothing happens.

Contributor

Mithgol commented Jun 28, 2013

@Dream707

im not sure, but chrome 28+ should have notifications http://blog.chromium.org/2013/05/rich-notifications-in-chrome.html

That's rich notifications for Chrome packaged apps and extensions. A different thing.

Here we are talking about HTML5 Notifications API.

The real issue here is that typeof Notification === 'function' already (in node-webkit 0.6.1) and that you can call new Notification('some test'), but nothing happens.

@YurySolovyov

This comment has been minimized.

Show comment
Hide comment
@YurySolovyov

YurySolovyov Jun 28, 2013

maybe just a flag somewhere in build config or something? or some permission issues(you have to ask a permission in order to display notification)

YurySolovyov commented Jun 28, 2013

maybe just a flag somewhere in build config or something? or some permission issues(you have to ask a permission in order to display notification)

@rogerwang

This comment has been minimized.

Show comment
Hide comment
@rogerwang

rogerwang Jun 30, 2013

Member

Hello, thanks for your patience.

The reason this feature is in Chromium but not in node-webkit is: The feature is implemented in the Chromium browser, which is built on Content Layer, on which is node-webkit built. (node-webkit is on the same level with the browser part of Chromium.) So code for this is not in node-webkit.

I planed to move it to node-webkit but recently it's under refactoring to a new component (message center) in upstream. The good news is that the new component is more Content Layer friendly so it would make the move easier. Now the plan is to provide this feature in node-webkit 0.7.x or 0.8.x if upstream makes it ready on all the 3 platforms.

Member

rogerwang commented Jun 30, 2013

Hello, thanks for your patience.

The reason this feature is in Chromium but not in node-webkit is: The feature is implemented in the Chromium browser, which is built on Content Layer, on which is node-webkit built. (node-webkit is on the same level with the browser part of Chromium.) So code for this is not in node-webkit.

I planed to move it to node-webkit but recently it's under refactoring to a new component (message center) in upstream. The good news is that the new component is more Content Layer friendly so it would make the move easier. Now the plan is to provide this feature in node-webkit 0.7.x or 0.8.x if upstream makes it ready on all the 3 platforms.

@Mithgol

This comment has been minimized.

Show comment
Hide comment
@Mithgol

Mithgol Jun 30, 2013

Contributor

Thank you for explaining the current state of things.

Contributor

Mithgol commented Jun 30, 2013

Thank you for explaining the current state of things.

@miklschmidt

This comment has been minimized.

Show comment
Hide comment
@miklschmidt

miklschmidt Jun 30, 2013

That sounds great @rogerwang thanks for the update!

miklschmidt commented Jun 30, 2013

That sounds great @rogerwang thanks for the update!

@damianb

This comment has been minimized.

Show comment
Hide comment
@damianb

damianb Jun 30, 2013

Please keep us posted - I guess we'll have to individually work around this and come up with some sort of temporary solution on our own.

For linux, at least, we've got notify-send.

damianb commented Jun 30, 2013

Please keep us posted - I guess we'll have to individually work around this and come up with some sort of temporary solution on our own.

For linux, at least, we've got notify-send.

@miklschmidt

This comment has been minimized.

Show comment
Hide comment
@miklschmidt

miklschmidt Jun 30, 2013

My extremely dependency heavy take at a workaround is here: https://gist.github.com/miklschmidt/5896306 if it's useful to anyone.

miklschmidt commented Jun 30, 2013

My extremely dependency heavy take at a workaround is here: https://gist.github.com/miklschmidt/5896306 if it's useful to anyone.

@damianb

This comment has been minimized.

Show comment
Hide comment
@damianb

damianb Jun 30, 2013

@miklschmidt That's quite helpful, thanks! I'll probably have to pick it over myself to make it a bit more agnostic, but that is a great start.

I take it vendor/tween is tween.js?

Also - what is lib/gui?

damianb commented Jun 30, 2013

@miklschmidt That's quite helpful, thanks! I'll probably have to pick it over myself to make it a bit more agnostic, but that is a great start.

I take it vendor/tween is tween.js?

Also - what is lib/gui?

@miklschmidt

This comment has been minimized.

Show comment
Hide comment
@miklschmidt

miklschmidt Jul 1, 2013

tween.js is TWEEN.js https://github.com/sole/tween.js/

lib/gui is just a require.js wrapper for require('nw.gui') i like to avoid using two types of dependency declarations in the same file :)

miklschmidt commented Jul 1, 2013

tween.js is TWEEN.js https://github.com/sole/tween.js/

lib/gui is just a require.js wrapper for require('nw.gui') i like to avoid using two types of dependency declarations in the same file :)

@vlados

This comment has been minimized.

Show comment
Hide comment
@vlados

vlados Aug 18, 2013

How we can help so this can be integrated as soon as possible?

vlados commented Aug 18, 2013

How we can help so this can be integrated as soon as possible?

@mbrevda

This comment has been minimized.

Show comment
Hide comment
@mbrevda

mbrevda commented Aug 26, 2013

+1

@jtg-gg

This comment has been minimized.

Show comment
Hide comment
@jtg-gg

jtg-gg Sep 3, 2014

Member

I agree with you @jamesmortensen, I've made a pull request #2289

Member

jtg-gg commented Sep 3, 2014

I agree with you @jamesmortensen, I've made a pull request #2289

@victorzimmer

This comment has been minimized.

Show comment
Hide comment
@victorzimmer

victorzimmer commented Sep 6, 2014

+1

@rogerwang rogerwang closed this Sep 10, 2014

@rogerwang rogerwang added the bounty label Sep 10, 2014

@adam-lynch

This comment has been minimized.

Show comment
Hide comment
@adam-lynch

adam-lynch Sep 10, 2014

Closed? Sorry, maybe I'm wrong, but are you settling for those tray tooltips on Windows?
image

IMO, they're not a good enough. Doesn't look like a real notification, can't show more than one at once, Windows is moving towards a different style notification since Windows 8, etc.

adam-lynch commented Sep 10, 2014

Closed? Sorry, maybe I'm wrong, but are you settling for those tray tooltips on Windows?
image

IMO, they're not a good enough. Doesn't look like a real notification, can't show more than one at once, Windows is moving towards a different style notification since Windows 8, etc.

@edjafarov

This comment has been minimized.

Show comment
Hide comment
@edjafarov

edjafarov Sep 16, 2014

Just published replacement for rich desktop notifications check live -http://screencast.com/t/bUxB6vNvW8BN

https://github.com/edjafarov/node-webkit-desktop-notification
html5 Notification like API, better animations.

edjafarov commented Sep 16, 2014

Just published replacement for rich desktop notifications check live -http://screencast.com/t/bUxB6vNvW8BN

https://github.com/edjafarov/node-webkit-desktop-notification
html5 Notification like API, better animations.

yoshuawuyts added a commit to yoshuawuyts/node-webkit-builder that referenced this issue Sep 19, 2014

gulpfile: update
Fixes an issue with node-webkit notifications -- nwjs/nw.js#27 (comment)
@edjafarov

This comment has been minimized.

Show comment
Hide comment
@edjafarov

edjafarov Sep 20, 2014

html5 notification ubuntu
screen shot 2014-09-20 at 8 33 33 pm

edjafarov commented Sep 20, 2014

html5 notification ubuntu
screen shot 2014-09-20 at 8 33 33 pm

@J-Rojas

This comment has been minimized.

Show comment
Hide comment
@J-Rojas

J-Rojas Sep 26, 2014

Hello, I've tried node-webkit v0.10.5 on Windows 7 (64 bit), Service Pack 1. When trying to create a new Notification, node-webkit crashed. This occurred with both a local script bundled into the application and also in an IFrame. Notifications on MacOSX and Ubuntu both worked.

If someone has had success on Windows 7, can you share what version of the OS and version of node-webkit worked for you. Thanks.

J-Rojas commented Sep 26, 2014

Hello, I've tried node-webkit v0.10.5 on Windows 7 (64 bit), Service Pack 1. When trying to create a new Notification, node-webkit crashed. This occurred with both a local script bundled into the application and also in an IFrame. Notifications on MacOSX and Ubuntu both worked.

If someone has had success on Windows 7, can you share what version of the OS and version of node-webkit worked for you. Thanks.

@sergiovilar

This comment has been minimized.

Show comment
Hide comment
@sergiovilar

sergiovilar commented Oct 15, 2014

@joshleaves @jamesmortensen @jtg-gg this works fine! Thanks!

adam-lynch added a commit to nwjs-community/nw-builder that referenced this issue Oct 29, 2014

gulpfile: update
Fixes an issue with node-webkit notifications -- nwjs/nw.js#27 (comment)

@rogerwang rogerwang changed the title from Implement desktop notifications [$100] to Implement desktop notifications Dec 15, 2014

@rogerwang rogerwang changed the title from Implement desktop notifications to Implement desktop notifications [$100 awarded] Feb 21, 2015

@marbemac

This comment has been minimized.

Show comment
Hide comment
@marbemac

marbemac Feb 24, 2015

Like @J-Rojas, I'm also seeing a crash in Windows 7 64 bit. The notifications are working fine in OS X and Linux. Does anybody have notifications working in Windows 7 (I have not tried other windows version yet)?

marbemac commented Feb 24, 2015

Like @J-Rojas, I'm also seeing a crash in Windows 7 64 bit. The notifications are working fine in OS X and Linux. Does anybody have notifications working in Windows 7 (I have not tried other windows version yet)?

@jtg-gg

This comment has been minimized.

Show comment
Hide comment
@jtg-gg

jtg-gg Feb 24, 2015

Member

@marbemac I've just tested the latest build, it works on my win 7 64 bit, hmm do you have the call stack for the crash ?

Member

jtg-gg commented Feb 24, 2015

@marbemac I've just tested the latest build, it works on my win 7 64 bit, hmm do you have the call stack for the crash ?

@marbemac

This comment has been minimized.

Show comment
Hide comment
@marbemac

marbemac Feb 24, 2015

@jtg-gg I checked the %TEMP% folder for the crash dump as described here https://github.com/nwjs/nw.js/wiki/Crash-dump but don't see anything. Is there somewhere else I should be looking?

When it crashes Windows only gives me a "programName.exe has stopped working. a problem caused the program to stop working correctly. Windows will close the program and notify you if a solution is available."

I took the notification code out to make sure, and sure enough the app booted fine. Then, in the dev console, I created a new notification while the app was running and it crashed.

marbemac commented Feb 24, 2015

@jtg-gg I checked the %TEMP% folder for the crash dump as described here https://github.com/nwjs/nw.js/wiki/Crash-dump but don't see anything. Is there somewhere else I should be looking?

When it crashes Windows only gives me a "programName.exe has stopped working. a problem caused the program to stop working correctly. Windows will close the program and notify you if a solution is available."

I took the notification code out to make sure, and sure enough the app booted fine. Then, in the dev console, I created a new notification while the app was running and it crashed.

@marbemac

This comment has been minimized.

Show comment
Hide comment
@marbemac

marbemac Feb 24, 2015

@jtg-gg I was able get the .dmp file. See this paste bin:

http://pastebin.com/869PPRku

Looks like something to do with storage.get?

[3664:0223/190703:FATAL:image.cc(748)] Check failed: storage_.get().

The code I'm using for the notification is just

new Notification("Title", {body: "Body"});

Thanks in advance for any help you can provide!

marbemac commented Feb 24, 2015

@jtg-gg I was able get the .dmp file. See this paste bin:

http://pastebin.com/869PPRku

Looks like something to do with storage.get?

[3664:0223/190703:FATAL:image.cc(748)] Check failed: storage_.get().

The code I'm using for the notification is just

new Notification("Title", {body: "Body"});

Thanks in advance for any help you can provide!

@jtg-gg

This comment has been minimized.

Show comment
Hide comment
@jtg-gg

jtg-gg Feb 24, 2015

Member

@marbemac looks like, there is problem of getting the application icon, the notification, will try to get the application icon, if no image is specified

Member

jtg-gg commented Feb 24, 2015

@marbemac looks like, there is problem of getting the application icon, the notification, will try to get the application icon, if no image is specified

@victorzimmer

This comment has been minimized.

Show comment
Hide comment
@victorzimmer

victorzimmer Feb 24, 2015

If it works on OSX and Linux, but not Windows with fetching an image from the file tree it sounds a lot like a path error :P


Victor Zimmer

On Tue, Feb 24, 2015 at 5:46 AM, jtg-gg notifications@github.com wrote:

@marbemac looks like, there is problem of getting the application icon, the notification, will try to get the application icon, if no image is specified

Reply to this email directly or view it on GitHub:
#27 (comment)

victorzimmer commented Feb 24, 2015

If it works on OSX and Linux, but not Windows with fetching an image from the file tree it sounds a lot like a path error :P


Victor Zimmer

On Tue, Feb 24, 2015 at 5:46 AM, jtg-gg notifications@github.com wrote:

@marbemac looks like, there is problem of getting the application icon, the notification, will try to get the application icon, if no image is specified

Reply to this email directly or view it on GitHub:
#27 (comment)

@marbemac

This comment has been minimized.

Show comment
Hide comment
@marbemac

marbemac Feb 24, 2015

Hmm, where should I set this application icon? Is it required? If it makes any difference, I'm currently using https://github.com/mllrsohn/node-webkit-builder to build the app.

marbemac commented Feb 24, 2015

Hmm, where should I set this application icon? Is it required? If it makes any difference, I'm currently using https://github.com/mllrsohn/node-webkit-builder to build the app.

@marbemac

This comment has been minimized.

Show comment
Hide comment
@marbemac

marbemac Feb 24, 2015

@victorzimmer @jtg-gg I set the winIco option in node-webkit-builder but no luck - still crashes. Any other ideas? Do Windows 7 (64) notifications work for you guys when you build with node-webkit-builder (just to rule that out)?

marbemac commented Feb 24, 2015

@victorzimmer @jtg-gg I set the winIco option in node-webkit-builder but no luck - still crashes. Any other ideas? Do Windows 7 (64) notifications work for you guys when you build with node-webkit-builder (just to rule that out)?

@jtg-gg

This comment has been minimized.

Show comment
Hide comment
@jtg-gg

jtg-gg Feb 25, 2015

Member

@marbemac can you try, to specify some jpg/png icon (http:// or file://) when you execute the notification ?

Member

jtg-gg commented Feb 25, 2015

@marbemac can you try, to specify some jpg/png icon (http:// or file://) when you execute the notification ?

@marbemac

This comment has been minimized.

Show comment
Hide comment
@marbemac

marbemac Feb 26, 2015

@jtg-gg I updated my notification code test to the below, and it works fine in OS X, but still crashes in Windows. Any other ideas? And it's working in Windows 7 for you? This is so weird. Incidentally I did try it on another windows 7 computer and it crashed on that one as well. I've also tried several of the packaging methods listed in the wiki.

var options = {
        icon: "http://www.fnordware.com/superpng/pngtest16rgba.png",
        body: "Here is the notification text"
};

var notification = new Notification("Notification Title", options);

marbemac commented Feb 26, 2015

@jtg-gg I updated my notification code test to the below, and it works fine in OS X, but still crashes in Windows. Any other ideas? And it's working in Windows 7 for you? This is so weird. Incidentally I did try it on another windows 7 computer and it crashed on that one as well. I've also tried several of the packaging methods listed in the wiki.

var options = {
        icon: "http://www.fnordware.com/superpng/pngtest16rgba.png",
        body: "Here is the notification text"
};

var notification = new Notification("Notification Title", options);
@marbemac

This comment has been minimized.

Show comment
Hide comment
@marbemac

marbemac commented Feb 26, 2015

Ok so the example here worked:
https://github.com/nwjs/nw.js/tree/master/tests/manual_tests/notification

Investigating..

@marbemac

This comment has been minimized.

Show comment
Hide comment
@marbemac

marbemac Feb 26, 2015

Solved, my manifest file had the window.icon set to a blank string. So many places to set an icon, I didn't notice that option in the manifest file. Thanks for the help guys!

marbemac commented Feb 26, 2015

Solved, my manifest file had the window.icon set to a blank string. So many places to set an icon, I didn't notice that option in the manifest file. Thanks for the help guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment