Sickchill: Multiple Chrome Notification

[DDA78]

Since some time i have observed that chrome has been showing multiple notifications for the sickrage, this happens with all the notifications, attached is the debug log & screenshot of check update notification I have received. I am not sure this is bug or expected behaviour.

Branch/Commit: Master/949e0acc613e8cc14c7555a3f154f031a94a9768
OS:Linux-4.9.16-v7+-armv7l-with-debian-8.0 (Raspberry PI 3)
What you did: Check Update & Other Notification like manual search of episodes
What happened: Getting Multiple Notification (2, 3, 4 even 5 sometimes)
What you expected: Get Single Notification
Logs:

2017-03-22 18:56:48 DEBUG Thread-16 :: check_for_new_news: Checking GitHub for latest news.
2017-03-22 18:56:48 INFO Thread-16 :: No update needed
2017-03-22 18:56:48 DEBUG Thread-16 :: cur_commit = 949e0acc613e8cc14c7555a3f154f031a94a9768, newest_commit = 949e0acc613e8cc14c7555a3f154f031a94a9768, num_commits_behind = 0, num_commits_ahead = 0
2017-03-22 18:56:48 DEBUG Thread-16 :: git rev-list --left-right "@{upstream}"...HEAD : returned successful
2017-03-22 18:56:48 DEBUG Thread-16 :: Executing git rev-list --left-right "@{upstream}"...HEAD with your shell in /opt/sickrage
2017-03-22 18:56:48 DEBUG Thread-16 :: git rev-parse --verify --quiet "@{upstream}" : returned successful
2017-03-22 18:56:48 DEBUG Thread-16 :: Executing git rev-parse --verify --quiet "@{upstream}" with your shell in /opt/sickrage
2017-03-22 18:56:48 DEBUG Thread-16 :: git branch --set-upstream-to origin/master : returned successful
2017-03-22 18:56:48 DEBUG Thread-16 :: Executing git branch --set-upstream-to origin/master with your shell in /opt/sickrage
2017-03-22 18:56:48 DEBUG Thread-16 :: git fetch origin : returned successful
2017-03-22 18:56:46 DEBUG Thread-16 :: Executing git fetch origin with your shell in /opt/sickrage
2017-03-22 18:56:46 DEBUG Thread-16 :: git config remote.origin.url https://github.com/SickRage/SickRage.git : returned successful
2017-03-22 18:56:46 DEBUG Thread-16 :: Executing git config remote.origin.url https://github.com/SickRage/SickRage.git with your shell in /opt/sickrage
2017-03-22 18:56:46 DEBUG Thread-16 :: git rev-parse HEAD : returned successful
2017-03-22 18:56:46 DEBUG Thread-16 :: Executing git rev-parse HEAD with your shell in /opt/sickrage
2017-03-22 18:56:46 DEBUG Thread-16 :: git symbolic-ref -q HEAD : returned successful
2017-03-22 18:56:46 DEBUG Thread-16 :: Executing git symbolic-ref -q HEAD with your shell in /opt/sickrage
2017-03-22 18:56:46 INFO Thread-16 :: Checking for updates using GIT
2017-03-22 18:56:22 DEBUG Thread-16 :: check_for_new_news: Checking GitHub for latest news.
2017-03-22 18:56:22 INFO Thread-16 :: No update needed
2017-03-22 18:56:22 DEBUG Thread-16 :: cur_commit = 949e0acc613e8cc14c7555a3f154f031a94a9768, newest_commit = 949e0acc613e8cc14c7555a3f154f031a94a9768, num_commits_behind = 0, num_commits_ahead = 0
2017-03-22 18:56:22 DEBUG Thread-16 :: git rev-list --left-right "@{upstream}"...HEAD : returned successful
2017-03-22 18:56:22 DEBUG Thread-16 :: Executing git rev-list --left-right "@{upstream}"...HEAD with your shell in /opt/sickrage
2017-03-22 18:56:22 DEBUG Thread-16 :: git rev-parse --verify --quiet "@{upstream}" : returned successful
2017-03-22 18:56:22 DEBUG Thread-16 :: Executing git rev-parse --verify --quiet "@{upstream}" with your shell in /opt/sickrage
2017-03-22 18:56:22 DEBUG Thread-16 :: git branch --set-upstream-to origin/master : returned successful
2017-03-22 18:56:22 DEBUG Thread-16 :: Executing git branch --set-upstream-to origin/master with your shell in /opt/sickrage
2017-03-22 18:56:22 DEBUG Thread-16 :: git fetch origin : returned successful
2017-03-22 18:56:20 DEBUG Thread-16 :: Executing git fetch origin with your shell in /opt/sickrage
2017-03-22 18:56:20 DEBUG Thread-16 :: git config remote.origin.url https://github.com/SickRage/SickRage.git : returned successful
2017-03-22 18:56:20 DEBUG Thread-16 :: Executing git config remote.origin.url https://github.com/SickRage/SickRage.git with your shell in /opt/sickrage
2017-03-22 18:56:20 DEBUG Thread-16 :: git rev-parse HEAD : returned successful
2017-03-22 18:56:20 DEBUG Thread-16 :: Executing git rev-parse HEAD with your shell in /opt/sickrage
2017-03-22 18:56:20 DEBUG Thread-16 :: git symbolic-ref -q HEAD : returned successful
2017-03-22 18:56:19 DEBUG Thread-16 :: Executing git symbolic-ref -q HEAD with your shell in /opt/sickrage
2017-03-22 18:56:19 INFO Thread-16 :: Checking for updates using GIT

[SickRage2]

Thanks for the issue report! Before a real human comes by, please make sure your report has all the below criteria checked

• [ ] Include basic information: Branch/Commit, OS, What you did, What happened, What you expected
• [ ] Enable debug logging (be sure to disable after the bug is fixed)
• [x] Post debug logs, either inline (for smaller logs) or using gist
• [x] Please wrap inline logs inside ``` and ``` for readability

Please make sure you also read how to create an issue and followed all of the steps.

The title should describe your issue. Having "SR not working" or "I get this bug" for 100 issues, isn't really helpful. We will close issues if there isn't enough information.

Sometimes the devs may seem like grunts and respond with short answers. This isn't (always) because the dev hates you, but because he's on mobile or busy fixing bugs. If something isn't clear, please let us know, and this bot may get updated to automatically answer you.

Thanks!

[sharkykh]

This is indeed an unexpected behavior. I don't have this issue, don't think I ever had.
I guess you tried restarting SickRage?
What browser are you using? Did you try with another browser?

[DDA78]

I have not tried another browser, I only use Chrome.

Have tried resetting the sickrage multiple times.

[sharkykh]

Confirmed:
This happens if you have multiple windows open, with a SickRage tab open in each window.
@DDA78 In order to avoid getting multiple notifications, use tabs instead of windows.
Open a new tab with Ctrl+T, instead of a new window with Ctrl+N

@SickRage/collaborators
I'm gonna look into Noty which looks really nice and supports mobile devices(!).
If it doesn't have this issue and works better (for instance, doesn't trim the notifcation text if it's too long), and if I can style it so that the notification has the SR logo too,
Then I think I'll replace the PNotify library with it.

[OmgImAlexis]

@sharkykh not 100% sure on this but I have a feeling this has something todo with the way sickrage handles sending out notifications. Ideally they should only be sent to a single window.
With tabs only one tab per window will have focus whereas multiple windows may have focus at once.

[OmgImAlexis]

Also personally I wouldn't even look at Noty as it seems it uses divs instead of browser's native notifications. If you want fake notifications then you can enable that in the library already being used. We actually disabled that functionally for this reason.

[sharkykh]

With tabs only one tab per window will have focus whereas multiple windows may have focus at once.

I didn't go too much into the code just yet, but I did see that section about window visibility.
So it seems plausible.

Also personally I wouldn't even look at Noty as it seems it uses divs instead of browser's native notifications. If you want fake notifications then you can enable that in the library already being used. We actually disabled that functionally for this reason.

The browser's native notifications' size is predetermined and feels too small for most of SR's notifications.
And like I said, it doesn't work om mobile browsers so you're not getting any notifications (I'm with Chrome on Galaxy S7) and it's confusing.

I noticed we are using pnotify<3.0, maybe version 3 fixes some of the issues without too much of a pain to migrate.

[sharkykh]

If you block the native notifications (when you deny permission via the browser) it should fallback to what you call "fake" notifications. Currently it shows no notifications, and just like on mobile, it's confusing.

[OmgImAlexis]

And like I said, it doesn't work om mobile browsers so you're not getting any notifications (I'm with Chrome on Galaxy S7) and it's confusing.

You really think on a screen that size in browser notifications would work? Think about the res and then the size the text inside the fake notification will need to be. @miigotu and I have discussed this before and that's why we never really changed it as there isn't a "good" way to do it. You've also got to keep in mind that a LOT of people use things like the iPhone which has a tiny res of only 750 x 1334 on a 4.7 inch screen.

If you block the native notifications (when you deny permission via the browser) it should fallback to what you call "fake" notifications. Currently it shows no notifications, and just like on mobile, it's confusing.

Nope nope nope nope nope! This would force something on a user when they've specifically told the application to not do the said thing. Forcing users to see notifications shouldn't happen.

Now I'm not saying this shouldn't be looked into I'm just saying it's not as easy as just dropping in another library.

[sharkykh]

You really think on a screen that size in browser notifications would work? Think about the res and then the size the text inside the fake notification will need to be.@miigotu and I have discussed this before and that's why we never really changed it as there isn't a "good" way to do it. You've also got to keep in mind that a LOT of people use things like the iPhone which has a tiny res of only 750 x 1334 on a 4.7 inch screen.

No, I didn't really think this through -
I just don't understand why the notifications don't show on the mobile Chrome, even though I allowed them for the SR address (checked again right now and it's enabled).

Nope nope nope nope nope! This would force something on a user when they've specifically told the application to not do the said thing. Forcing users to see notifications shouldn't happen.

IMO we need to show something, somewhere.
How can you know the result of a manual search? Or the progress of a version update (backup result)?
It doesn't sit right with me.

What if we give the user the option to choose if they want to display non-native notifications instead of blocking them altogether?

Now I'm not saying this shouldn't be looked into I'm just saying it's not as easy as just dropping in another library.

It's all good.
We're just making conversation :)

[sharkykh]

I guess the mobile notifications issue doesn't originate from SR itself. As this isn't working for me either. :/

[OmgImAlexis]

I just don't understand why the notifications don't show on the mobile Chrome, even though I allowed them for the SR address (checked again right now and it's enabled).

http://caniuse.com/#feat=notifications <-- That's why

IMO we need to show something, somewhere.
How can you know the result of a manual search? Or the progress of a version update (backup result)?

If the user has said they don't want notifications then we should accept that. Notifications are really meant for things that don't show visual feedback. Saving a config refreshes the page and shows the newly saved details, doing a manual search shows the results on the page the user is still on, etc. so it's really not the biggest issue when you think of how they're "meant" to be used.

[miigotu]

could add modals to create "Done!" windows, lol....

Really a small overlay in the bottom right of the window that fades in and says "saved" for a few seconds would stop a lot of complaints. But that should only be if we cant send native notifications and we dont have an easy way to know if the user blocked them.

[sharkykh]

I think I found an elegant fix for the issue, by adding the desktop: { tag: 'unique_id' } (docs) option to each notification.
I can pass name from here to here as the unique tag.
name equals notification-<id> where <id> is the unique message number received from localhost:8081/ui/get_messages.

But...
I just checked again and I can't reproduce it the same way I did earlier (on 949e0acc613e8cc14c7555a3f154f031a94a9768), so I can't test if it's really working.

@DDA78
Are you still having the issue?

[DDA78]

@sharkykh

yes, i am still facing the issue. I still receive the multiple notifications

[sharkykh]

@DDA78 Can you checkout the bugfix/multiple-notifications branch, and see if it fixes the issue for you?

[DDA78]

@sharkykh

I am traveling out of station for a week, will try when on site

thanks for your help for working on the issue

[sharkykh]

@DDA78 Any news on this?