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

Make KDE use Freedesktop portal #2495

Merged
merged 1 commit into from
Apr 5, 2022
Merged

Make KDE use Freedesktop portal #2495

merged 1 commit into from
Apr 5, 2022

Conversation

greenfoo
Copy link
Contributor

This fixes #2436

It looks like in the newest KDE, using the Fredesktop portal is the only way that works (at least that seems to be the case with Fedora 35, where I have tested this change).

Having said so... it could be that this ticket breaks compatibility with older versions of KDE (after all, the special KDE code this PR is replacing was originally put in there for a reason!).

So... there are two options here: either merge this PR and forget about backwards compatibility or create a more sofisticated logic which (for the KDE case) tries with different approaches until one succeeds (KDE Screenshot / KDE Screenshot2 / Freedesktop / ...).

I'm not personally a fan of looking backwards... but you decide :)

After this change, all 3 supported desktops (GNOME, KDE and Sway) use the Freedesktop portal... and I suppose we could even go as far as dropping the Desktop check from this function and always try to use the Freedesktop portal as this will probably also work with others.

@greenfoo greenfoo changed the title Make KDE use Freddesktop portal Make KDE use Freedesktop portal Mar 20, 2022
@borgmanJeremy
Copy link
Contributor

Couple of questions since I'm having trouble testing on wayland right now.

  1. When using the portal do you get the prompt to "share" the screen with flameshot?
  2. Why not use https://invent.kde.org/plasma/kwin/-/blob/master/src/effects/screenshot/org.kde.KWin.ScreenShot2.xml

@greenfoo
Copy link
Contributor Author

  1. When using the portal do you get the prompt to "share" the screen with flameshot?

No :) It just takes the screenshot without asking.
This is also the case in Sway (ie. wlroots based compositors).

Out of curiosity I've been looking at the source code of xdg-desktop-portal and this is how it seems to work:

  1. There is a "frontend" component in charge of receiving dbus requests (ie. "org.freedesktop.portal.Desktop")
  2. When a request to take a screenshot is received, it is forwarded to a "backend" implementation depending on the running compositor:
    1. The "backend" implementation for GNOME seems to always show the pop-up asking the user whether he wants to allow a screenshot to be taken (only "native" GNOME applications, such as the built in screenshot tool, can bypass this dialog using an internal GNOME API (?)).
    2. The "backend" implementation for KDE seems to never show the pop-up. I've read (can't find the link now) that it shouldn't be possible and that you shoud grant explicit permissions to the app trying to take the screenshot by placing a special string ("X-KDE-DBUS-Restricted-Interfaces=...") on the app .desktop file, but I found no trace of this limitation in the code and was able to run flameshot without any extra pop-up (maybe it only applies to the older org.kde.KWin.ScreenShot{,2} interfaces?)
    3. The "backend" implementation for Sway/wlroots seems to never show the pop-up either. It simply ends up running "grim" which uses one experimental Wayland protocol (only supported by wlroots it seems) to take the screenshot without asking for confirmation.
  1. Why not use https://invent.kde.org/plasma/kwin/-/blob/master/src/effects/screenshot/org.kde.KWin.ScreenShot2.xml

It looks like the most recent "consensus" is to move to the Freedesktop interface, which is considered "the way to go in the future" (or, at least, that's what they end up doing in Gimp's screenshot tool):

gimp

Notice, however, that at the end they still kept the KDE portal in case the Freedesktop one failed (instead of completely dropping it). Something similar could be done here... or not, considering the Freedesktop portal should be available in all except very old distros.

@borgmanJeremy
Copy link
Contributor

Thanks for documenting all that! It's really helpful to have it all summarized here.

Since this code path only is activated on Wayland, it makes sense to me to just use the Freedesktop implementation. If users are on old distros they are likely using X11.

@mmahmoudian any objections before I merge this?

@mmahmoudian
Copy link
Member

there are two options here: either merge this PR and forget about backwards compatibility or create a more sofisticated logic which (for the KDE case) tries with different approaches until one succeeds (KDE Screenshot / KDE Screenshot2 / Freedesktop / ...).

I'm not personally a fan of looking backwards... but you decide :)

Well, in spite of my preference on being on the latest version of software, we should see which if the functionality breaks in major KDE-based point-based release distros i.e kubuntu, Fedora KDE spin. I will try these two distros today and will report back.

One question that comes to mind is that of this backwards compatibility can be mitigated by using Flatpak or Snap.

In general I have no objections and I support the cause, but we should identify which major popular distros will break and we create a clear announcement to avoid confusion and avalanche of new bug reports.

@borgmanJeremy
Copy link
Contributor

Thanks I tested this on Fedora36 beta and this change is required.

@borgmanJeremy borgmanJeremy merged commit 3cd2dec into flameshot-org:master Apr 5, 2022
@borgmanJeremy
Copy link
Contributor

This also worked on Arch, but importantly I had to uninstall gnome first. There was some conflict while having the xdg portals of both gnome and kde installed at the same time.

panpuchkov pushed a commit to namecheap/flameshot that referenced this pull request May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wayland does not support QWindow::requestActivate() on KDE Plasma
3 participants