xdg-desktop-portal icon indicating copy to clipboard operation
xdg-desktop-portal copied to clipboard

Launching by a shortcut stopped working for flameshot (bisected to utils: Match all valid App IDs from systemd unit names)

Open darkblaze69 opened this issue 2 years ago • 45 comments

Arch Linux, GNOME 44, 45 beta

I set shortcut in GNOME to launch 'flameshot gui' to Super+Print. After updating XDP to 1.17 the shortcut stopped working.

Bisected to d4063fc00a070c26659c6a56a1d175da96048212 (utils: Match all valid App IDs from systemd unit names). What should be improved from flameshot side? How the issue with an App ID could be diagnosed?

flameshot is not a flatpak.

darkblaze69 avatar Aug 08 '23 18:08 darkblaze69

GNOME doesn't support the global shortcut portal, yet. I'm very confused how you seemingly managed to get a bisected commit here.

swick avatar Aug 09 '23 12:08 swick

I installed Flameshot (the flatpak version) to test.

I set the shortcut in Settings, the only way to launch an application from GNOME with keyboard shortcuts. The command I used is flatpak run org.flameshot.Flameshot gui

The application launches; this can be checked in background apps. From what I remember, Flameshot is supposed to show the screenshot portal, then take a screenshot, then show its GUI (the drawing tools on the screenshot, etc). Also, Flameshot didn't work for a long time. There were issues about Flamshot not being able to take screenshots with the portal. Note that ksnip, another screenshot tool also seems unable to use the portal (I saw that there is access to the d-bus interface, so I guess it also uses the screenshot portal). So maybe it's about displaying the screenshot portal UI, not keyboard shortcuts per se.

Mikenux avatar Aug 10 '23 17:08 Mikenux

When trying to launch flameshot gui by shortcut I get this in log:

Aug 25 01:35:00 systemd[1536]: Started Application launched by gsd-media-keys.
Aug 25 01:35:00 xdg-desktop-por[29983]: g_app_info_get_display_name: assertion 'G_IS_APP_INFO (appinfo)' failed
Aug 25 01:35:00 xdg-desktop-por[29983]: Failed to show access dialog: GDBus.Error:org.freedesktop.DBus.Error.AccessDenied: Only the focused app is allowed to show a system access dialog
Aug 25 01:35:00 flameshot[30397]: flameshot: error: Unable to capture screen

I have reverted the commit d4063fc00a070c26659c6a56a1d175da96048212 on top of version 1.17 and it works normal now, no error in log.

So some new regex1 + regex2 logic cannot work it. For flameshot when it works I got a scope like app-gnome-flameshot-6780.scope if it helps. What else could I provide to debug the behavior?

here's a patch I apply on 1.17: regex-my.patch.txt

darkblaze69 avatar Aug 24 '23 20:08 darkblaze69

Alright, this is starting to make sense to me. The borken regex didn't match the scope before and now it does. This is not about the global shortcut portal but about another portal failing when flameshot is launched.

swick avatar Aug 24 '23 21:08 swick

Is it fixable on xdg-desktop-portal side?

darkblaze69 avatar Aug 24 '23 21:08 darkblaze69

One more question: does the desktop file 'flameshot.desktop' exist? There is definitely a bug in the screenshot code that gets triggered by us matching on more app ids.

swick avatar Aug 24 '23 21:08 swick

This file /usr/share/applications/org.flameshot.Flameshot.desktop

darkblaze69 avatar Aug 24 '23 21:08 darkblaze69

Yeah, that explains how it manages to trigger it. The scope says just 'flameshot' but the desktop file is 'org.flameshot.Flameshot.desktop'. If you start flameshot from the shell and not via shortcut, is the .scope different? Wondering what's going on here.

Anyway, for the actual bug in the screenshot portal, can you try this patch?

diff --git a/src/screenshot.c b/src/screenshot.c
index 91d69e8..215d208 100644
--- a/src/screenshot.c
+++ b/src/screenshot.c
@@ -247,7 +247,10 @@ handle_screenshot_in_thread_func (GTask *task,
 
           id = g_strconcat (app_id, ".desktop", NULL);
           info = g_desktop_app_info_new (id);
-          name = g_app_info_get_display_name (G_APP_INFO (info));
+          if (info)
+            name = g_app_info_get_display_name (G_APP_INFO (info));
+          else
+            name = app_id;
 
           title = g_strdup_printf (_("Allow %s to Take Screenshots?"), name);
           subtitle = g_strdup_printf (_("%s wants to be able to take screenshots at any time."), name);

swick avatar Aug 24 '23 21:08 swick

No luck, now it times out.

Aug 25 03:00:38 systemd[1526]: Started Application launched by gsd-media-keys.
Aug 25 03:00:45 xdg-desktop-por[2005]: Failed to show access dialog: Timeout was reached
Aug 25 03:00:45 flameshot[2353]: flameshot: error: Unable to capture screen
Aug 25 03:00:45 flameshot[2353]: flameshot: error: Unable to capture screen

darkblaze69 avatar Aug 24 '23 22:08 darkblaze69

oh boy... more bugs. I'll need a more info:

  • exactly how the shortcut was created (from settings? with a command? the exact command!)
  • the contents of every desktop file with flameshot in the name anywhere on the system
  • the backend that's handling the screenshot request (ps x|grep xdg-desktop-portal)
  • is something in any logs about xdg-desktop-portal-something processes crashing?
  • which version of everything are the portal and backend

swick avatar Aug 24 '23 22:08 swick

I can't test anything for now (only from Monday), but the screenshot portal is (when I tested) functional with the ASHPD Demo app (flatpak version; https://flathub.org/apps/com.belmoussaoui.ashpd.demo). This is on Fedora 38 (xdg-desktop-portal-gnome-44.2-1, xdg-desktop-portal-1.16.0-3).

Mikenux avatar Aug 24 '23 22:08 Mikenux

  • Shortcut created by GNOME Settings > Keyboard > Custom Shortcuts

Screenshot from 2023-08-25 03-30-14

  • When run from command line flameshot gui I don't see any flameshot scopes, even tried it with watch -n.1 : systemctl --user --all | grep -i flameshot.

  • When succeeds (XDP 1.16) the scope looks like app-gnome-flameshot-12505.scope Loaded active running Application launched by gsd-media-keys

  • Only one system desktop (checked by find / -iname *flameshot*desktop 2>/dev/null) /usr/share/applications/org.flameshot.Flameshot.desktop (renamed to txt for github to accept): org.flameshot.Flameshot.desktop.txt

  • portals:

ps x | grep [x]dg-desktop-portal
   9518 ?        Ssl    0:00 /usr/lib/xdg-desktop-portal
   9686 ?        Ssl    0:00 /usr/lib/xdg-desktop-portal-gnome
   9747 ?        Ssl    0:00 /usr/lib/xdg-desktop-portal-gtk
  • versions:
xdg-desktop-portal 1.17.0-1
xdg-desktop-portal-gtk 1.14.1-1
xdg-desktop-portal-gnome 45beta-1 (44.3 also affected)

darkblaze69 avatar Aug 24 '23 22:08 darkblaze69

what happens when you launch via gtk-launch org.flameshot.Flameshot.desktop?

swick avatar Aug 24 '23 22:08 swick

With this command I suppose it just launches binary as in default entry, I see it in ps x | grep flameshot after that. But I need it with option flameshot gui to take the needed 'Desktop Action Capture'.

darkblaze69 avatar Aug 24 '23 23:08 darkblaze69

Alright, I have a complete picture now.

The command in the shortcut (flameshot) doesn't match the desktop file (org.flameshot.Flameshot.desktop). This means that gsd-media-keys fails to get the correct app id (org.flameshot.Flameshot) and results in the scope app-gnome-flameshot-6780.scope.

We might be able to correspond the command to the right desktop file with a bit more effort in g-m-k, but you're using flatpak this would always work just fine. Not sure if it's worth the effort. Maybe we could change a bit how shortcuts work and instead of just a command let the user specify the application ( + applications actions, + arguments) which would fix this issue as well.

With my fix it doesn't really matter that we're not able to identify the app correctly, it just means the permissions between launching from the shell and launching via shortcut are different.

Either way, this now all works as intended.

The Failed to show access dialog: Timeout was reached message is the result of a bug that was fixed already (1f966bfb8f59fe25875dcc481a17f70bcbc435d4). The Failed to show access dialog: GDBus.Error:org.freedesktop.DBus.Error.AccessDenied: Only the focused app is allowed to show a system access dialog message is on purpose because flameshot has focus because it has no window.

Not sure if there is something we want to do here.

swick avatar Aug 24 '23 23:08 swick

In the 1st description I said I don't use flatpak, flameshot is a standalone package.

darkblaze69 avatar Aug 25 '23 00:08 darkblaze69

I know. Flameshot should name the desktop file matching the executable name, i.e. 'flameshot.desktop' (obviously that doesn't apply to the flatpak version). You can open an issue on their bug tracker.

Then there is the patch above which I'm going to put into a MR after I checked a few other places in the code for the same mistake.

And lastly, the error message you're seeing is on purpose. Flameshot needs to create a window before using the portal. You should also open an issue on their bug tracker.

swick avatar Aug 25 '23 13:08 swick

https://github.com/flatpak/xdg-desktop-portal/pull/1087

I think this is the only actionable thing in xdg-desktop-portal. For everything else you have to talk with flameshot.

swick avatar Aug 25 '23 14:08 swick

Naming the executable so it matches the desktop file is not really how it's supposed to work, the desktop file basename should match the D-Bus name. So org.flameshot.Flameshot.desktop is more correct. If the stuff that tries to guess the app id makes a wrong guess, the method of figuring out should be fixed, instead of renaming the desktop file incorrectly.

jadahl avatar Aug 25 '23 14:08 jadahl

https://github.com/flameshot-org/flameshot/issues/3213

Mikenux avatar Aug 25 '23 15:08 Mikenux

Wasn't it a screenshot sharing window before it was a dialog asking for permission? Wasn't this turned into a permission dialog because users didn't want to have to share the screenshot every time, so it was decided to go with a "always" permission instead of having "this time" and "always" options?

Mikenux avatar Aug 25 '23 16:08 Mikenux

@jadahl it is started by a string that's interpreted as a command. Not sure how you want to go from there to the desktop file other than by name. Maybe the gnome keyboard shortcut thing should let users select a desktop file with an optional run command so that gnome-media-keys can actually start the program in the right scope.

swick avatar Aug 27 '23 14:08 swick

I have the same issue on Ubuntu 23.10 with Wayland. From command line flameshot gui works as expected.

When using Gnome Shorcut (/usr/bin/flamshot gui), I can see this error in syslog: dg-desktop-por[10352]: Failed to show access dialog: GDBus.Error:org.freedesktop.DBus.Error.AccessDenied: Only the focused app is allowed to show a system access dialog

heyyo-droid avatar Oct 09 '23 10:10 heyyo-droid

@swick wouldn't it be feasible to scan through the available desktop files to see if there is a unique desktop file that has a matching Exec ? Or alternatively matching some other attribute in the desktop file.

pschichtel avatar Oct 10 '23 18:10 pschichtel

I found a workaround from a user's perspective.

I could see when I ran Flameshot from the terminal.

Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.

So, Flameshot works but needs that QT_QPA_PLATFORM variable to be wayland.

In Gnome custom shortkeys, I tried all of these and it didn't work.

1. QT_QPA_PLATFORM=wayland && flameshot gui
2. QT_QPA_PLATFORM=wayland; flameshot gui
3. QT_QPA_PLATFORM=wayland in .zshrc

The actual workaround was to write a bash script and put it in $HOME/.local/bin

  1. nano takescreenshotfull
  2. #!/bin/bash
    
    QT_QPA_PLATFORM=wayland
    flameshot full --path <screenshots_dir>
    
  3. chmod +x $HOME/.local/bin/takescreenshotfull
  4. In Gnome custom shortkeys, fill in command field with takescreenshotfull

wtechgo avatar Oct 14 '23 12:10 wtechgo

Flameshot explictely mess around with the QPA platform https://github.com/flameshot-org/flameshot/blob/0bbb9528615c1d3697e0538c4a53d8d0e00ade0a/src/main.cpp#L42

so the joke is on them.

hfiguiere avatar Oct 14 '23 15:10 hfiguiere

I found a workaround from a user's perspective.

I could see when I ran Flameshot from the terminal.

Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.

So, Flameshot works but needs that QT_QPA_PLATFORM variable to be wayland.

In Gnome custom shortkeys, I tried all of these and it didn't work.

1. QT_QPA_PLATFORM=wayland && flameshot gui
2. QT_QPA_PLATFORM=wayland; flameshot gui
3. QT_QPA_PLATFORM=wayland in .zshrc

The actual workaround was to write a bash script and put it in $HOME/.local/bin

  1. nano takescreenshotfull
  2. #!/bin/bash
    
    QT_QPA_PLATFORM=wayland
    flameshot full --path <screenshots_dir>
    
  3. chmod +x $HOME/.local/bin/takescreenshotfull
  4. In Gnome custom shortkeys, fill in command field with takescreenshotfull

I am running flameshot on Fedora 39, and it does not show the warning and also run under Wayland by default.

but strange thing is, your tricks just works for me, I change it to: env QT_QPA_PLATFORM=wayland flameshot gui

now the custom keymaping works again

the command:

sudo tee /usr/local/bin/flameshot-gui-workaround > /dev/null <<'EOF'
#!/bin/bash
# workaround thanks to https://github.com/flatpak/xdg-desktop-portal/issues/1070#issuecomment-1762884545
env QT_QPA_PLATFORM=wayland flameshot gui

EOF

sudo chmod a+x /usr/local/bin/flameshot-gui-workaround

image

ttys3 avatar Oct 19 '23 16:10 ttys3

So, Flameshot works but needs that QT_QPA_PLATFORM variable to be wayland. In Gnome custom shortkeys, I tried all of these and it didn't work.

1. QT_QPA_PLATFORM=wayland && flameshot gui
2. QT_QPA_PLATFORM=wayland; flameshot gui
3. QT_QPA_PLATFORM=wayland in .zshrc

IIRC the correct syntax is QT_QPA_PLATFORM=wayland flameshot gui. That's how you set an environment variable for a single command.

kekonn avatar Oct 20 '23 12:10 kekonn

@ttys3

cat <<EOF > test
#!/bin/bash
# workaround thanks to https://github.com/flatpak/xdg-desktop-portal/issues/1070\#issuecomment-1762884545
env QT_QPA_PLATFORM=wayland flameshot gui
EOF

@kekonn Thanks for sharing the correct IIRC syntax, however env QT_QPA_PLATFORM=wayland flameshot gui didn't work for me.

wtechgo avatar Oct 22 '23 14:10 wtechgo

The env in front of it probably makes the difference. I was just talking syntax, not the entire solution. My bad :smile:

kekonn avatar Oct 22 '23 15:10 kekonn