search

telegramdesktop / tdesktop

Telegram Desktop messaging app
https://desktop.telegram.org/
Other
26.11k stars 5.18k forks source link

[Warning] qt.qpa.wayland: Wayland does not support QWindow::requestActivate() #23873

Closed mazunki closed 2 years ago

mazunki commented 2 years ago

Steps to reproduce

  1. Set QT_QPA_PLATFORM to either of wayland, wayland-egl.
  2. Open telegram-desktop
  3. Go to a chat, and press to display an image, right click anything, or try to forward something
  4. See warning in terminal

Expected behaviour

No warning.

I'm not sure what it should be activating, since I don't feel anything is wrong.

Actual behaviour

Several warnings saying qt.qpa.wayland: Wayland does not support QWindow::requestActivate().

Operating system

Gentoo Linux with Sway

Version of Telegram Desktop

3.4.3

Installation source

Other (unofficial) source

Note, the official Gentoo .ebuild for telegram-desktop essentially uses the official https://github.com/telegramdesktop/tdesktop/releases/download/v${PV}/${MY_P}.tar.gz tarball for installation, so it's likely an upstream issue.

Logs

telegram-desktop

(telegram-desktop:7573): Telegram-WARNING **: 18:16:25.734: Application was built without embedded fonts, this may lead to font issues.
qt.svg: Error while inflating gzip file: SVG format check failed
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
Corrupt JPEG data: premature end of data segment
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
Invalid return value 0 for stream protocol
Invalid return value 0 for stream protocol
ilya-fedin commented 2 years ago

Yeah, Qt doesn't support that, this is not a tdesktop bug

mazunki commented 2 years ago

I'll throw a report over at them, too, then. Meanwhile, can we disable the warning by preemptively checking, and just throw an initial "Disabling requestActivate because it is not supported by Qt" warning?

ilya-fedin commented 2 years ago

This is printed by Qt, not tdesktop, tdesktop has no control over that

mazunki commented 2 years ago

It's only printed by Qt if we try to invoke it, or am I wrong? If that's the case, wouldn't the solution be to never invoke it if we know it won't work? I was reading a bit about it, and it seems it's a limitation of Wayland itself, by design. If that's the case, I feel we should handle it properly.

ilya-fedin commented 2 years ago

If that's the case, wouldn't the solution be to never invoke it if we know it won't work?

I don't think a lot of code should be changed just because of Wayland

If that's the case, I feel we should handle it properly.

Wayland should eventually add window activation support or that would cause a lot of problems like e.g. impossibility to activate the window after clicking on notification, activate the window when clicking on a link or activate the window when clicking on the tray icon.

mazunki commented 2 years ago

I don't think a lot of code should be changed just because of Wayland

Wayland is not some niche modern thing of hipsters anymore, it is the default in many widely used distributions:

Wayland should eventually add window activation support or that would cause a lot of problems like e.g. impossibility to activate the window after clicking on notification, activate the window when clicking on a link or activate the window when clicking on the tray icon.

Wayland is designed from a security perspective: Applications shouldn't be allowed to control other applications (like grabbing the attention of other windows). This is unlike X11, where applications just trust applications aren't malicious.

While I agree it would be nice if Qt, or the Wayland protocol itself had support for this kind of permission-granting... and I expect it to eventually become a thing, it would regardless require some extra work to implement support for it. We may as well split it apart already.

ilya-fedin commented 2 years ago

Wayland is not some niche modern thing of hipsters anymore, it is the default in many widely used distributions:

X11 users are still the biggest part of Linux tdesktop users according to @john-preston's statistics (>50% are Ubuntu X11 users). And Linux is only 1.82% of entire tdesktop userbase. So, Wayland is still a niche thing (less than 1% of entire tdesktop userbase), sorry.

Wayland is designed from a security perspective: Applications shouldn't be allowed to control other applications (like grabbing the attention of other windows). This is unlike X11, where applications just trust applications aren't malicious.

These things won't ever work correctly, then. And the warning is valid, then.

mazunki commented 2 years ago

These things won't ever work correctly, then. And the warning is valid, then.

Whenever Qt and/or Wayland decide to implement a permission based system, Telegram will have to grant permission for applications to put their windows in focus, informing Wayland about it, and notification applets will have to request the window manager through Wayland that they want Telegram to be in focus.

This is still not a thing, but whenever it happens, we might as well be ready. That's my point.

X11 users are still the biggest part of Linux tdesktop users according to @john-preston's statistics

Because people don't update their systems very often. It is still the default in new systems. You're right in that Linux itself is kind of niche.

I might look into implementing some groundwork myself, if nobody else is up for it, but I don't feel like rejecting it as an issue is a good solution.

ilya-fedin commented 2 years ago

This is still not a thing, but whenever it happens, we might as well be ready. That's my point.

I don't know how silenting the warnings would make it ready

mazunki commented 2 years ago

I'm not familiar enough with Telegram's codebase to answer straight up.

My first idea is to check at what points it is called, and create a wrapper for calling the function. Inside the wrapper, we check if we're using X11 or Wayland. If we're using X11, then just invoke it as usual, and return true. If we're under Wayland, just return false for now.

ilya-fedin commented 2 years ago

So you want to wrap calls here and there through the codebase just to silent a warning that seeing less than 1% (and if we split XWayland users who don't actually see it then it will be ~0.1%) of users? Are you seriously?

mazunki commented 2 years ago

You make it sound like having wrappers is a bad idea. I don't understand why that's the case when it can avoid duplicate code due to difference across platforms.

Furthermore, for now it might only silence warnings that nobody sees (because, let's be real, most people don't even look at their logs), but eventually it will be necessary to handle focus requests properly.

Again, I might look into it myself. I'm not asking for anyone to do it, because at the end of the day it's not causing me any problems, but it's still a warning that should be fixed.

ilya-fedin commented 2 years ago

I believe it's better to not touch the calls at that point (until you have thoughts on how to fix something concrete) because:

  1. When Qt implements activation support, the wrappers may become unneeded
  2. No one knows how the API would look like (if it would need additional calls), so the wrapper may not fit the architecture required for that API

It's better to make the changes when it would be clear what and how to change.

mazunki commented 2 years ago

1) Fair point. I'm not sure if Qt has spoken about it. Would be nice if they decided on something. 2) Even then, I'm assuming X11 support would want to be kept. In that case, it would still be useful to have a wrapper to handle each case individually.

I'm pretty confident in that, even if/when Qt adds support for activation requests, they would still require an interface to manage it, up to each application to configure, as that's the intention of Wayland.

ilya-fedin commented 2 years ago

2. Even then, I'm assuming X11 support would want to be kept. In that case, it would still be useful to have a wrapper to handle each case individually.

Maybe you didn't understand, I meant the wrapper you create right now may not fit the architecture of the future API and another rewrite would be needed.

mazunki commented 2 years ago

Are you referring to the signature of the functions used? That's an important consideration, and I think you're right now in that we should wait until Qt says something.

Anyway, I would like to add some use cases for the security aspects: 1) A notification daemon would like to open Telegram when you click on it. Sounds good to me. 2) You're playing a game, and don't want to be disturbed. You thus tell your window manager to disable all permissions of apps to show themselves out of nowhere (but you want to keep your keyboard bindings/shortcuts). 3) You get a call, and you're not in "do not disturb" mode on your window manager. It should show itself. 4) A malicious program wants to take a screenshot of a chat by activating it, or to send a message. We shouldn't let all program take control of Telegram, only those we/the users trust.

ilya-fedin commented 2 years ago

Are you referring to the signature of the functions used?

It's the most obvious thing, but not the only. I can also imagine that not all calls would need to be changed.