Skip to content
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

notification is undefined #65

Closed
DiscoTim opened this issue Aug 11, 2016 · 8 comments
Closed

notification is undefined #65

DiscoTim opened this issue Aug 11, 2016 · 8 comments

Comments

@DiscoTim
Copy link

DiscoTim commented Aug 11, 2016

i receive this error in FF on Win 10 when i click the notification that pops up on the demo site: https://nickersoft.github.io/push.js/

TypeError: notification is undefined
Push/closeNotification()push.min.js:97
Push/createCallback/wrapper.close()push.min.js:245
Push/createCallback/<()push.min.js:252
1push.min.js:97:1

which is line 97 in the non-minified script:

if (notification.close) {

@Nickersoft
Copy link
Owner

Checking just now, I'm not getting this issue on FF 47 (on OSX El Capitan), but I have Windows 10 duel-booted on my Mac back at home, so I'll definitely investigate later today. I've marked it as high priority. Seems something might be fishy going on with how Push is handling notification IDs.

@DiscoTim
Copy link
Author

i noticed this error in my own app, so i went to check on your demo, and same error. i'm using FF 48 on win 10. i am going to double check with a longer timeout, but it seems the time of the error corresponds to that. ie.. click the notifications and it closes, then the error appears when the timeout would have expired.

@DiscoTim
Copy link
Author

DiscoTim commented Aug 11, 2016

the timing of the error corresponds to the timeout. the error only occurs if the notification is clicked and closed, and for example.. if there is a 15 second timeout the error will appear after 15 seconds.

also, same error in chrome on win 10.

is it getting destroyed twice? once on the click and removed then the timeout tries to remove it again?

@Nickersoft
Copy link
Owner

Yeah, I agree. When the timeout expires, it calls notification.close(), which is where the issue is occurring. notification is being pulled from the global list of notifications (the same one that Push.count() uses), which must not be functioning properly.

@Nickersoft
Copy link
Owner

So I finally got a chance to test this on Firefox 48 on Windows 10 and MacOS Sierra, and I able to confirm the issue. I think you are correct, it is most likely being destroyed twice. I'll see what I can do to fix it.

@Nickersoft
Copy link
Owner

Commit 26387ab resolves this issue using a fix similar to the one mentioned in #66. Closing this ticket.

@DiscoTim
Copy link
Author

thats excellent! thank you very much. will you be pushing this to cdnjs? the current version there is 0.0.10

@Nickersoft
Copy link
Owner

Yup! Very soon :) Just wanted to wait a bit to make sure no bugs were being reported with the new code changes.

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

No branches or pull requests

2 participants