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

Add an option to display notification content even if screen is locked #27666

Open
GranPC opened this issue Mar 30, 2024 · 14 comments
Open

Add an option to display notification content even if screen is locked #27666

GranPC opened this issue Mar 30, 2024 · 14 comments

Comments

@GranPC
Copy link

GranPC commented Mar 30, 2024

Is your feature request related to a problem?

Telegram Desktop currently checks whether the screen is locked before displaying notifications. If it is, notification content is redacted and looks like this:

image

While this makes sense from a privacy perspective, some operating systems actually have system-wide controls for that:

image

The system-wide setting ends up providing a better UX: unlocking the device after receiving a notification can actually cause that notification's content to be unveiled.

Describe the solution you'd like

Ideally we would have a toggle inside Notification settings to bypass this behavior. It could be called "Show message content on lock screen" or similar.

Describe alternatives you've considered

The only alternative I have come up with so far - which worked - was to patch the Telegram binary to disable the check and always show notification content. This immediately improved the UX significantly.

Additional context

I am running Telegram Desktop on a Linux phone running phosh. I am also working on improvements to the shell itself, and notifications are a focus of mine right now.

Delegating the decision of when and where to display notification content would enable some interesting use cases that are currently not possible, such as smartwatch notification support and priority contacts.

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Mar 30, 2024

Ideally we would have a toggle inside Notification settings to bypass this behavior. It could be called "Show message content on lock screen" or similar.

Adding new settings is usually a strong no (needs a permission from @john-preston who usually says no). Although an automatic behavior of detecting & following the system-wide setting would be acceptable I guess (it's up to @john-preston). The only thing is, tdesktop has no one from the Phosh community to impelement that, do you want to work on implementing that or maybe you can ask someone from your community to help tdesktop on that? If not, that will remain unimplemented I'm afraid.

@GranPC
Copy link
Author

GranPC commented Mar 30, 2024

I am fine with looking into it myself and submitting a patch.

One problem with detecting the existence of the system-wide setting is that even though the gsettings schema may exist, it is not actually guaranteed that the compositor and the notification daemon will make use of it. In fact, the currently released version of Phosh does not; so there's no fool-proof way to truly autodetect it.

I do understand the reasoning behind not adding more settings than necessary, but on the other hand, I can see this feature being useful in other scenarios. For example, notifications are not displayed on the lock screen in elementary OS, which means that if messages are received while the screen is locked, the user ends up receiving a barrage of useless notifications whenever they unlock the screen.

Honestly I'm not even 100% sure masking notification content should be Telegram Desktop's responsibility in any case. DEs, notification daemons and operating systems should do that by default, and I believe they already do in most if not all cases. I guess we'd just have to know why the feature was introduced in the first place.

@ilya-fedin
Copy link
Contributor

One problem with detecting the existence of the system-wide setting is that even though the gsettings schema may exist, it is not actually guaranteed that the compositor and the notification daemon will make use of it.

Can we get this standardized in freedesktop then? Some flag whose presence means the DE supports hiding the content of notifications and tdesktop can not do that.

Honestly I'm not even 100% sure masking notification content should be Telegram Desktop's responsibility in any case. DEs, notification daemons and operating systems should do that by default, and I believe they already do in most if not all cases. I guess we'd just have to know why the feature was introduced in the first place.

Well, this is cross-platform code and tdesktop prefers to manage the notifications client-side, as the UX of custom notifications is considered better for most of users and there are even periodic bug reports from the users that they can't enable them on Wayland, so they seem to really like it. There were no Linux implementation before #26256, though.

@GranPC
Copy link
Author

GranPC commented Mar 30, 2024

I have no idea how the FreeDesktop standardization process works. I assume we'd want it to be defined as a server capability, which Telegram Desktop would query to figure out the correct behavior, but I have no idea how to get the ball rolling on that.

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Mar 30, 2024

It seems the spec is abandoned? I made an issue two years ago and a PR a year ago, still no reaction... There seem to be some work on portals, though. I even proposed to let applications provide custom text on lock screen but it was rejected. tdesktop would have no way to use it by default, though, as tdesktop has a custom FDO spec implementation and experimental GNotification code, the latter is opt-in under an experimental setting due to this nasty glib bug that allows to support either only FDO or only portal but not both (so relying fully on it would mean bug reports from either half of the user base).

I will ask @john-preston about whether Core::App().screenIsLocked() is really meant to be used with native notifications, though. Maybe it's inherently applied or maybe Windows/macOS APIs mean the check should be done on app side.

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Mar 31, 2024

Hm, I noticed Core::App().screenIsLocked() is in NativeManager::forceHideDetails(), so is used only for native notifications. This perhaps means that custom notifications don't appear over locked screen on any OS while native notifications do and the OSes don't really automatically hide content? Given #26256, that is apparently right for Linux as well.

@ilya-fedin
Copy link
Contributor

Yeah, it is indeed meant to hide the content only on native notifications: e26e666. This means the situation is not as good as you thought and OSes don't automatically hide the content.

@ilya-fedin
Copy link
Contributor

Given that "show message content on lock screen" is enabled by default on you screenshot, phosh seem to have the problem just as well?

@GranPC
Copy link
Author

GranPC commented Apr 3, 2024

OSes don't automatically hide the content.

I'm pretty sure Windows, macOS and some (many? most? dunno!) Linux desktop environments do. But yeah, we definitely need to either make it a setting, or figure out a way for the system to declare that it hides the content and the app needn't worry. I am trying to look into the second thing.

Given that "show message content on lock screen" is enabled by default on you screenshot, phosh seem to have the problem just as well?

It's not enabled by default; I enabled it myself. That toggle isn't actually hooked up to anything just yet though, and phosh hides notification contents in the lockscreen no matter what.

@agx
Copy link

agx commented Apr 3, 2024

OSes don't automatically hide the content.

I'm pretty sure Windows, macOS and some (many? most? dunno!) Linux desktop environments do. But yeah, we definitely need to either make it a setting, or figure out a way for the system to declare that it hides the content and the app needn't worry. I am trying to look into the second thing.

Given that "show message content on lock screen" is enabled by default on you screenshot, phosh seem to have the problem just as well?

It's not enabled by default; I enabled it myself. That toggle isn't actually hooked up to anything just yet though, and phosh hides notification contents in the lockscreen no matter what.

I have an unfinished show-on-lockscreen branch for here from 2021 here. IIRC the main question was what to do with actions in that case.

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Apr 3, 2024

I'm pretty sure Windows, macOS and some (many? most? dunno!) Linux desktop environments do.

Well, @john-preston says this was likely added due to macOS.

It's not enabled by default; I enabled it myself. That toggle isn't actually hooked up to anything just yet though, and phosh hides notification contents in the lockscreen no matter what.

That's apparently the problem, even if it would be hooked up: the user expects the private information to not be visible on lock screen out of the box, not after going to settings.

@GranPC
Copy link
Author

GranPC commented Apr 3, 2024

That's apparently the problem, even if it would be hooked up: the user expects the private information to not be visible on lock screen out of the box, not after going to settings.

The default, at least on Phosh, is to hide message content.

@ilya-fedin
Copy link
Contributor

The default, at least on Phosh, is to hide message content.

Ah, ok

@WPMGPRoSToTeMa
Copy link

WPMGPRoSToTeMa commented May 11, 2024

Adding new settings is usually a strong no

Even if it's under Experimental settings? I'm a bit tired of seeing these faceless (and useless) "You have a new message" notifications when I wake my PC from sleep/hibernate. As a workaround I have to disconnect the PC from the Internet, wake it up, unlock, then connect it back to the Internet, and finally I can see the notifications normally.

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

4 participants