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

[windows] Show warning when system has denied Notifications access #1884

Closed
1 task done
GuardianMajor opened this issue Dec 9, 2017 · 6 comments
Closed
1 task done

Comments

@GuardianMajor
Copy link

GuardianMajor commented Dec 9, 2017

  • I have searched open and closed issues for duplicates

NOTE: I found a few issues referencing the minimized to tray state but nothing specifically for the notification issue as experienced here, even though some do involve notifications. Feel free to merge if my assessment was wrong.

In response to: #1676 (comment)

I think it's time to take some of this behavior to other issues. This pull request is long-dead.

I had to specifically search for this feature. I saw that most of the activity was within the last 30 days and up to the last 24 hours so I felt since it was regarding that feature it was an appropriate place, didn't want to add to the noise with another issue. My intention was not to continue in a dead topic or whatnot, hope you understand.

In particular, your item 3 is definitely a bug. Notifications are supposed to happen when the window is minimized. We'll need more information: platform details, log, etc. as usual.

It is good to know it is a bug, so this issue will be able to address that then. I had no way to know it was a bug, just figured it was a feature limitation. Now there is context, we can figure it out.


Bug description

When the desktop beta client is minimized to tray, there are no notification shown when new messages arrive.

Steps to reproduce

  • Minimize to tray on start or manually by closing the app (assumes command line options)
  • Ask someone to send you a message
  • Nothing pops up to notify you of a new message has arrived

Actual result: No notifications shown when new messages arrive while minimized to tray.
Expected result: Some type of notification should be shown, be it system/os native notification or slide/toast notification, and/or even ideally the icon on the tray changing to show a badge icon of new message count.

Platform info

Operating System: Windows 10 Pro 1709.16299.64
Signal version: 1.1.0-beta.5

Link to debug log

https://gist.github.com/anonymous/b0fbffa325bb01f52150975d46a4c25e

@scottnonnenberg
Copy link
Contributor

This may be a windows-specific issue. I just tested it on MacOS and it worked as expected. And I believe it was originally developed on Linux. Will try it on windows soon.

@GuardianMajor
Copy link
Author

@scottnonnenberg I will be happy to help you test this or figure out, just let me know what you need from me and it is yours. You have access to me on the number on Signal and also private email, whatever works for you, or here, that's fine too. Just let me know if/what/when/how.

@scottnonnenberg
Copy link
Contributor

I just tried this on Windows 10 with v1.1.0-beta.5 and I did see notifications even when the window was hidden to the tray.

@GuardianMajor looking at your log, we are attempting to create the notifications. You're seeing notifications when you have the window visible, right? I know windows 10 has an notification-suppressing 'quiet hours' feature, maybe that's blocking the notifications?

@GuardianMajor
Copy link
Author

No, I get no notifications be it visible or not. I never have, something I always felt was lacking.

I saw what I presumed to be attempts in the log as well, but they are not succeeding and not producing an error either, so that's something of a problem in itself. I have been trying to see how it is trying to interact with the notification service on Windows, because I believe you guys didn't account for systems where notification access is denied to non-system apps, which would mean that it will not be able to create the notification. That's why, while it is always nice to have native notifications, there needs to be in the least a backup when such system is no available or accessible. I believe this is why most apps will not rely on the native notification because they can't guarantee or ensure they have requisite access to it.

@scottnonnenberg
Copy link
Contributor

@GuardianMajor Sorry, I'm confused. You've disabled notifications at the OS level for Signal but expect us to work around that somehow?

@GuardianMajor
Copy link
Author

GuardianMajor commented Dec 16, 2017

@scottnonnenberg before continuing with the hostile tone which frankly I cannot understand why, be clear that not all systems are configured the way the development test box is configured. You can't expect that everyone has an open system with every feature turned on and no development can reasonably expect this in the ecosystem.

This is the reason why most, if not all, messenger services in the class produce their own notification without relying on the OS, because you can't expect that 1) everyone will have the same configuration, 2) that they want to have the configuration forced on them to make it work, 3) that you should rely on something you cannot properly verify even exists.

In the very least, you need to check to see if the notification service is available, if it is, then by all means use it to push it, but if not then produce a notification which is already built into the framework you are using to build the desktop client. Or adopt the best practice of not assuming that a given feature will always been available on all systems and produce a solution, such as your own notifications, that can be consistently and always produced no matter what.

Many systems have this and other things disabled or restricted by policy, a software cannot expect to make the user choose to supersede those policies or live with a broken functionality that is albeit quite integral to the functionality of the product. So no, I didn't disable anything and expect you to make it work, it was already disabled and not specifically to Signal, as it doesn't even show up on the notification access list, when you added the feature without considering how it will fail when the service you are relying on doesn't exist.

Side note: Since you closed the other issue to stifle discussion which is counter to principle of open source community driven development, it is not a matter of wanting or whatever and yes prioritization matters, but not when the most secure messaging software in the world is leaving the very secure client open and exposed without taking even the most basic steps to secure it. The feature exists on Android and if that pin/timeout system is robust enough, then on OS like Windows, the password and vault store will be sufficiently more robust than nothing at all and it is a not a want or can or should feature, it is quite a reasonable and prudent solution to secure the client. Even if you don't wan to implement a super fancy system, which I don't think any suggestion made constituted that, then at the very least a basic password, however "weak" you may consider it is stronger than the nothing that secures it now.

Anyway, hopefully we can get back to figuring out the problem and a better solution than trying to blame the user which is quite contrary to principle of open source community driven discussion and contribution, which is not always someone who submits code, without users and testers, who are you building for?

@scottnonnenberg scottnonnenberg changed the title [electron/beta][windows] No Notification When Minimized to Tray [windows] Show warning when system has denied Notifications access Dec 18, 2017
@signalapp signalapp locked and limited conversation to collaborators Dec 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

3 participants