QubesOS / qubes-issues

The Qubes OS Project issue tracker
https://www.qubes-os.org/doc/issue-tracking/
542 stars 48 forks source link

[Contribution] qvm-screenshot-tool #953

Open marmarek opened 9 years ago

marmarek commented 9 years ago

Community Dev: @ben-grande (originally @evadogstar) Repo: https://github.com/ben-grande/qubes-qvm-screenshot-tool


Reported by axon on 16 Feb 2015 11:02 UTC The ability send dom0 screenshots directly to an AppVM/DispVM from the screenshot app is an oft-requested feature (see below). The ability to transfer saved screenshot files from dom0 to other VMs is already available, but it is neither obvious nor easy for most users to do this.

User requests/queries about this: https://groups.google.com/d/topic/qubes-devel/m8TfyvSqvf4/discussion https://groups.google.com/d/topic/qubes-devel/CwSPqtPYTRQ/discussion https://groups.google.com/d/topic/qubes-devel/_a7KxHbkSJo/discussion https://groups.google.com/d/topic/qubes-users/l6vOqhsd7ss/discussion https://groups.google.com/d/topic/qubes-users/etxwrc6rsIM/discussion https://groups.google.com/d/topic/qubes-users/_7FzKv6eJqA/discussion

Migrated-From: https://wiki.qubes-os.org/ticket/953

ninavizz commented 4 years ago

^ ...and only pinging Marta, because Marek is already spread too thin... but if Marek is the better person for this, then I'll gladly work with Mr Marek on this!

marmarek commented 4 years ago

See screenshots on https://github.com/evadogstar/qvm-screenshot-tool The plan is to connect it to PrintScr button as a default action, instead of the current xfce4-screenshooter. The UI for making screenshots is not that nice as in xfce4-screenshooter, but on the other hand, it allows to copy the image out of dom0 to various places (selected vm, imgur upload etc) out of the box.

ninavizz commented 4 years ago

Yeah, not having to use the CLI is a pretty big deal. I also really love that it allows for annotations.

I previously saw those, and they're rad—but they lack the context to let me know what was invoked to show which, and how it all works. If that makes sense. Full screenshots are more helpful than the tidy little windows that are in that repo.

Below is kinda where I'm at with noodling on Tray icon/functionality stuff. It feels like this would be the best way imho to invoke the screenshot tool (from the tray vs from a menu)—but I don't know what the invoking would prompt. I also don't feel that Tray icons need to invoke dropdown menus—if anything I feel it'd be more helpful if they did not longer-term. But the context needs to suit those usability things.

Thoughts?

image

marmarta commented 4 years ago

In the future we will be moving more and more towards removing things from dom0 - some tools will be in GUI VM (like most widgets, including things like batter widgets), some tools in Audio VM (like volume controls). I wonder if we should make such things also distinguished visually, or just consider those "administrative VMs" and not color their widgets.

v6ak commented 4 years ago

Maybe it depends:

marmarek commented 4 years ago

I haven't tried yet, but probably some changes will be needed. One difference may be some qrexec confirmation prompts and/or required policy changes (when calling qvm-run from dom0, you don't have prompts). To avoid this, it may be required to implement some of the actions (like the upload helper) as a dedicated qrexec service, and setup policy appropriately. Also, copying image should be done with qvm-copy-to-vm, not qvm-run.

fepitre commented 4 years ago

scrot is a dead package since Fedora 32. We cannot install/use it under latest R4.1.

tlaurion commented 4 years ago

@fepitre @marmarek

[user@dom0 ~]$ sudo qubes-dom0-update qubes-repo-contrib
[user@dom0 ~]$ sudo qubes-dom0-update qvm-screenshot-tools

No packages avail. Testing because not moved over to stable (testing phase is how long?)

[user@dom0 ~]$ sudo qubes-dom0-update --enablerepo=qubes-contrib-dom0-r4.0-current-testing qvm-screenshot-tool
Using sys-whonix as UpdateVM to download updates for Dom0; this may take some time...
--> Running transaction check
---> Package qvm-screenshot-tool.x86_64 0:0.7-0.fc25 will be installed
--> Processing Dependency: scrot for package: qvm-screenshot-tool-0.7-0.fc25.x86_64
--> Running transaction check
---> Package scrot.x86_64 0:0.8-14.fc24 will be installed
--> Processing Dependency: libImlib2.so.1()(64bit) for package: scrot-0.8-14.fc24.x86_64
--> Processing Dependency: libgiblib.so.1()(64bit) for package: scrot-0.8-14.fc24.x86_64
--> Running transaction check
---> Package giblib.x86_64 0:1.2.4-24.fc24 will be installed
---> Package imlib2.x86_64 0:1.4.9-1.fc25 will be installed
--> Finished Dependency Resolution
qvm-screenshot-tool-0.7-0.fc25.x86_64.rpm                   |  18 kB  00:00     
/var/lib/qubes/dom0-updates/packages/imlib2-1.4.9-1.fc25.x86_64.rpm already exists and appears to be complete
/var/lib/qubes/dom0-updates/packages/giblib-1.2.4-24.fc24.x86_64.rpm already exists and appears to be complete
/var/lib/qubes/dom0-updates/packages/scrot-0.8-14.fc24.x86_64.rpm already exists and appears to be complete
Successfully verified /var/lib/qubes/dom0-updates/packages/giblib-1.2.4-24.fc24.x86_64.rpm
Successfully verified /var/lib/qubes/dom0-updates/packages/imlib2-1.4.9-1.fc25.x86_64.rpm
Successfully verified /var/lib/qubes/dom0-updates/packages/qubes-repo-contrib-4.0.6-1.fc25.noarch.rpm
ERROR: could not verify /var/lib/qubes/dom0-updates/packages/qvm-screenshot-tool-0.7-0.fc25.x86_64.rpm
Successfully verified /var/lib/qubes/dom0-updates/packages/scrot-0.8-14.fc24.x86_64.rpm
ERROR: could not verify one or more packages
marmarek commented 4 years ago

Maybe contrib key is not imported? Try sudo rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY-qubes-4-contrib-fedora

tlaurion commented 4 years ago

@marmarek

[user@dom0 ~]$ sudo rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY-qubes-4-contrib-fedora 
[user@dom0 ~]$ sudo qubes-dom0-update --enablerepo=qubes-contrib-dom0-r4.0-current-testing qvm-screenshot-tool
Using sys-whonix as UpdateVM to download updates for Dom0; this may take some time...
--> Running transaction check
---> Package qvm-screenshot-tool.x86_64 0:0.7-0.fc25 will be installed
--> Processing Dependency: scrot for package: qvm-screenshot-tool-0.7-0.fc25.x86_64
--> Running transaction check
---> Package scrot.x86_64 0:0.8-14.fc24 will be installed
--> Processing Dependency: libImlib2.so.1()(64bit) for package: scrot-0.8-14.fc24.x86_64
--> Processing Dependency: libgiblib.so.1()(64bit) for package: scrot-0.8-14.fc24.x86_64
--> Running transaction check
---> Package giblib.x86_64 0:1.2.4-24.fc24 will be installed
---> Package imlib2.x86_64 0:1.4.9-1.fc25 will be installed
--> Finished Dependency Resolution
qvm-screenshot-tool-0.7-0.fc25.x86_64.rpm                   |  18 kB  00:00     
/var/lib/qubes/dom0-updates/packages/imlib2-1.4.9-1.fc25.x86_64.rpm already exists and appears to be complete
/var/lib/qubes/dom0-updates/packages/giblib-1.2.4-24.fc24.x86_64.rpm already exists and appears to be complete
/var/lib/qubes/dom0-updates/packages/scrot-0.8-14.fc24.x86_64.rpm already exists and appears to be complete
Successfully verified /var/lib/qubes/dom0-updates/packages/giblib-1.2.4-24.fc24.x86_64.rpm
Successfully verified /var/lib/qubes/dom0-updates/packages/imlib2-1.4.9-1.fc25.x86_64.rpm
Successfully verified /var/lib/qubes/dom0-updates/packages/qubes-repo-contrib-4.0.6-1.fc25.noarch.rpm
ERROR: could not verify /var/lib/qubes/dom0-updates/packages/qvm-screenshot-tool-0.7-0.fc25.x86_64.rpm
Successfully verified /var/lib/qubes/dom0-updates/packages/scrot-0.8-14.fc24.x86_64.rpm
ERROR: could not verify one or more packages
TheOne-Z commented 4 years ago

@marmarek @tlaurion I replicated this and it seems that you cannot verify any packages from qubes-repo-contrib either, at least in dom0.

andrewdavidwong commented 4 years ago

@marmarek @tlaurion I replicated this and it seems that you cannot verify any packages from qubes-repo-contrib either, at least in dom0.

This seems like a more general bug, so I've opened #6124 for it.

icequbes1 commented 4 years ago

Maybe contrib key is not imported? Try sudo rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY-qubes-4-contrib-fedora

This is essentially the issue - the updatevm doesn't get the new key from etc/yum.repos.d/qubes-contrib-dom0-r4.0.repo when running qubes-dom0-update the second time.

Quick hack: kill /var/lib/qubes/dom0-updates in the updatevm (sys-whonix) after installing qubes-repo-contrib, then install qvm-screenshot-tool.

e.g.,

# add contrib repo
sudo qubes-dom0-update qubes-repo-contrib
# clear updatevm dom0 updates cache
qvm-run sys-whonix 'sudo rm -rf /var/lib/qubes/dom0-updates'
# install contrib packages
sudo qubes-dom0-update --enablerepo=qubes-contrib-dom0-r4.0-current-testing qvm-screenshot-tool

Edit: perhaps adding --clean on the second invocation of qubes-dom0-update would remove the need for the rm command within sys-whonix for a cleaner alternative.

tlaurion commented 4 years ago

@marmarek @fepitre @marmarta : Wouldn't it be possible to change the alt-printscreen and printscreen shortcuts in XFCE to ease UX at installation of package? Advanced users can once again do that themselves, while I consider this contrib to be a total needed tool for all users and should be the default, even more if added customizations are still needed prior of usage.

2020-10-17-132510

marmarek commented 4 years ago

Totally makes sense. There is a little technical issue: Xfce copies some of the configuration at first start and at this point there is no longer a place for system-wide default that is easy to override by the user - there is only that user place. So, setting the default works only before the very first user login (after initial install). Changing that later requires quite elaborate hack. As linked - we've done both before, but I'd like some less hacky solution...

dylangerdaly commented 4 years ago

What's the latest with this? Scrot isn't available for R4.1

dylangerdaly commented 4 years ago

Are we going to maintain scrot, and by extension libgiblib?

marmarek commented 4 years ago

See discussion at https://github.com/resurrecting-open-source-projects/scrot/issues/22 I don't think maintaining scrot is the way to go - if the above won't be resolved, I'd rather consider using some other maintained tool instead - there doesn't seem to be much required from it, shouldn't be that hard to switch.

dylangerdaly commented 4 years ago

Fair enough, I'll keep an eye out.

jpouellet commented 4 years ago

I (and a couple others) still use https://github.com/jpouellet/qubes-screenshot-helper regularly. It's clean, minimal, and integrates with the default xfce4-screenshooter tool which is both still maintained and already included in dom0 by default.

All that should be necessary for adoption is forking the repo under an official Qubes org (-contrib wasn't ready for use when this was written, nor do I really think working screenshots should be some awkward 3rd-party experience for users) and inclusion/installation of the package in the default installer iso.

It could optionally be improved to use the default qubes-RPC UI (instead of a Zenity dialog) for selecting the target VM in which to save the screenshot, but, IIRC this was annoying to implement at the time due to the qubes-RPC dialog's tight coupling with actual performing of RPC. Maybe it'd be easier today, I haven't looked. I haven't even thought about this in quite a while since the tool continues to "Just Work" as-is since I wrote it.

marmarek commented 4 years ago

Done, qubes-snapshot-helper is added to standard repos for R4.0 and R4.1 - current-testing for now.

dylangerdaly commented 4 years ago

*qubes-screenshot-helper is added to standard repos for R4.0 and R4.1 - current-testing for now.

fepitre commented 3 years ago

Look like it is possible to change scrot in my tool to maim. https://github.com/naelstrof/maim

I can do that at next version, but... not sure now.

1. is it available at fedora 33? It is on fedora 32.

2. I can't test it, because I'm on old Qubes... and there is no maim in the current repo to test it...

You can try in an AppVM to see how maim is working. Then, simply use this feedback to implement it in your code. I/anyone would be able to test it on latest devel R4.1.

And there is already 0.8 version on my repo of qvm-screenshot-tool

Don't worry about version. Just do the modification code into a separate branch. Test and validate from your side and open a PR into https://github.com/QubesOS-contrib/qubes-qvm-screenshot-tool.

fepitre commented 3 years ago

@evadogstar I've tested to screenshot a portion of window and sent it to appvm. It works. If I understand, maim is not available for Fedora 25? If not, I would need to create a branch on QubesOS-contrib repository. Do you mind to create a PR please?

fepitre commented 3 years ago

I think we are going to do a little wrapper or something to check which one to use. It's better than doing branches and pretty easy even from package point of view. Don't worry about PR, I'll cherry pick your commits and finish the rest.

fepitre commented 3 years ago

Does anyone can test this please: https://github.com/QubesOS-contrib/qubes-qvm-screenshot-tool/pull/1. I've refactored a little bit and tested it briefly under 4.1 between two cups of coffee.

fepitre commented 3 years ago

@evadogstar version is bumped after merge of new features/fixes. Current PR is only for fixing scrot/maim handling. We are going to bump to something like 0.7.1. When you will have done your new features/major changes we will bump it to probably 1.X. Don't worry about version, you can add features etc in separate branches and we take care of version. Also I would recommend you to sync your master branch with the QubesOS-contrib and then create new features/fixes from it. It helps to merge your work directly. If you need some help with git stuff you can ask here. You can also have a look at https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/proposing-changes-to-your-work-with-pull-requests.

Che15ea commented 2 years ago

Hi Most of my friends complain and wish to give up at the point of experiencing the complication of screenshot function while it can be much easier without compromising any security. I think it helps significantly to implement crop function when the screenshot is made, and a GUI dialog box to send to a selected VM or a default VM. Despite that I use alias script to simplify the process. But it still takes many unnecessary steps to crop and send a picture: screenshot - use terminal to move to vm - crop with GIMP - send to another vm - share

tlaurion commented 2 years ago

Hi Most of my friends complain and wish to give up at the point of experiencing the complication of screenshot function while it can be much easier without compromising any security. I think it helps significantly to implement crop function when the screenshot is made, and a GUI dialog box to send to a selected VM or a default VM. Despite that I use alias script to simplify the process. But it still takes many unnecessary steps to crop and send a picture: screenshot - use terminal to move to vm - crop with GIMP - send to another vm - share

@Che15ea Not sure I follow. With now provided through contrib repo's qvm-screenshot-tool (used to provide that screenshot below through alt-printscreen, then selecting region on screen, then selecting destination vm directly from the tool, and passing that screenshot saved under that qube's pictures directory from a file manager, with latest timestamp, directly to my browser's redacting region): 2022-09-21-170211

It's a matter of defining proper shortcuts (XFCE, Gnome or KDE) for alt-printscreen key combination and have it use mouse selected region as a screenshot. Above for XFCE which i'm using. Something else missing?

@marmarek : The tools is so reliable I would pledge to have it deployed as default and have the keyboard's shortcuts deployed as default as well. That is what is provided in Insurgo's OEM disk image for past years.

andrewdavidwong commented 2 years ago

If it's already available, should this issue be closed?

Che15ea commented 2 years ago

Hi @tlaurion @andrewdavidwong Thank you for your reply! Is it feasible to make it more newbie friendly and fool proof? Like pressing "PrintScreen" key and popping up crop and send-to-vm function by default without the need of installing anything or running any code. I am a geek and I am OK with current implementation but it is so difficult for my journalist and activist friends to adapt the current Screenshot function. I don't think adding command in keyboard shortcut or manually installing the is newbie friendly. It needs lots of research to implement the qvm-screenshot-tool. Sincerely

tlaurion commented 2 years ago

@andrewdavidwong

From OP "The ability send dom0 screenshots directly to an AppVM/DispVM from the screenshot app is an oft-requested feature (see below). The ability to transfer saved screenshot files from dom0 to other VMs is already available, but it is neither obvious nor easy for most users to do this."

It is still geeky to deploy, is not deployed as default and requires users to customize things.

As @Che15ea just replied, I do not think original goals of opened ticket are met. If that is a goal, then the default printscreen and at-printscreen shortcuts should be already customized and qvm-screenshot-tool deployed by default under Q4.2?

Che15ea commented 2 years ago

@andrewdavidwong

From OP "The ability send dom0 screenshots directly to an AppVM/DispVM from the screenshot app is an oft-requested feature (see below). The ability to transfer saved screenshot files from dom0 to other VMs is already available, but it is neither obvious nor easy for most users to do this."

It is still geeky to deploy, is not deployed as default and requires users to customize things.

As @Che15ea just replied, I do not think original goals of opened ticket are met. If that is a goal, then the default printscreen and at-printscreen shortcuts should be already customized and qvm-screenshot-tool deployed by default under Q4.2?

I think so, crop and send to vm is a function that everyone needs and all my journalist and activist friends start to be frustrated about Qubes at this point. It should not take every user's 20 minutes to setup.

DemiMarie commented 2 years ago

@Che15ea I agree with this. I think this should be installed by default. @marmarta what do you think?

unman commented 2 years ago

Once again, different experiences. The people I support don't have issues with copying files out of dom0 , regardless of their level of experience. They tend NOT to use this package, and just use a screen shot tool in dom0.

lorenzog commented 2 years ago

Cross-posting here (just noticed this thread, apologies for the duplicate) from https://github.com/QubesOS/qubes-issues/issues/5809#issuecomment-1286192955

TL;DR

Save this in dom0 and make executable:

#!/bin/sh
# find current window
WINDOW_ID=$(xprop -root _NET_ACTIVE_WINDOW | cut -d ' ' -f 5 | tr -d ',')
TARGET_VM=$(xprop -id $WINDOW_ID _QUBES_VMNAME | cut -d ' ' -f 3 | tr -d ''')
# save to clipboard, using rectangular selection
xfce4-screenshotter -c -r
# send to TARGET
xclip -out -sel c -t image/png | qvm-run --pass-io $TARGET_VM 'xclip -sel c -t image/png'

Go to settings and map the 'print screen' key to the script above. This will send the screenshot to the VM in focus when pressing 'print screen'. I'm still looking for a solution to have a nice dialog to ask the user which VM they want to send the screenshot to - something like the dialog for 'qvm-copy'.

UndeadDevel commented 1 year ago

For anyone who is having trouble getting @lorenzog 's code to work (e.g. xclip not installed by default on Qubes 4.1 dom0 and the method of grabbing focused VM throws syntax error for me), here is a different version:

#!/bin/sh
# lets the user take a screenshot based on rectangular selection and sends it to the currently focused VM

CUR_WIN_ID=`xdotool getwindowfocus`
CUR_VM=`xprop _QUBES_VMNAME -id $CUR_WIN_ID | cut -d \" -f 2`

if [[ "$CUR_VM" != "_QUBES_VMNAME:  not found." ]]; then
    xfce4-screenshooter -r -o "qvm-copy-to-vm $CUR_VM"
    notify-send "Screenshot sent!" "Your selection has been sent as a screenshot to $CUR_VM!"
fi
lorenzog commented 1 year ago

For anyone who is having trouble getting @lorenzog 's code to work (e.g. xclip not installed by default on Qubes 4.1 dom0 and the method of grabbing focused VM throws syntax error for me), here is a different version:

Hi! Thanks for the update - in fact, as you noticed xclip stopped working since the ability to copy images with mime-type image/png disappeared from the latest version. There's a deep rabbit hole that I haven't got into yet fully and probably never will.

I can't seem to get your script to work though - mind you, I'm using i3 and not xfce at the moment, but the -o option doesn't seem to pass the output to the target VM.

lorenzog commented 1 year ago

Ok, I went down the rabbit hole :) this script now works in Qubes 4.1. It requires xclip to be present on the destination qube.

#!/bin/sh
WINDOW_ID=$(xprop -root _NET_ACTIVE_WINDOW | cut -d ' ' -f 5 | tr -d ',')
echo "Window ID: $WINDOW_ID"
TARGET_VM=$(xprop -id $WINDOW_ID _QUBES_VMNAME | cut -d ' ' -f 3 | tr -d '"' )
echo "Target VM: $TARGET_VM"
DEST=$(mktemp)
echo "Dest: $DEST"
xfce4-screenshooter -r -s "$DEST"
cat "$DEST" | qvm-run --pass-io $TARGET_VM -- xclip -sel c -t image/png -l 1
rm "$DEST"

What needed to change:

  1. xclip was "waiting" for the receiver (qvm-run) to pick up the content of the clipboard with the appropriate "target", and because qvm-run never asked for a target, it was returning an error (and an empty clipboard)
  2. The screenshot is now saved to a temporary location on disk (mkstemp is not available in dom0?)
  3. The -l 1 instructs xclip to only listen for one selection (e.g. your target application in the qube) before closing itself. This means that screenshots won't be available more than once, unless you run a clipboard manager like parcellite.
UndeadDevel commented 1 year ago

I can't seem to get your script to work though - mind you, I'm using i3 and not xfce at the moment, but the -o option doesn't seem to pass the output to the target VM.

Works perfectly for me on a pretty fresh Q4.1 install and doesn't require installing any packages... It would be nice if the much more feature-rich tool was included by default in the qubes repo, though. I just learned about QubesOS-contrib packages...so never mind.

CumpsD commented 10 months ago

I'm still looking for a solution to have a nice dialog to ask the user which VM they want to send the screenshot to - something like the dialog for 'qvm-copy'.

This would be an awesome addition. Or the ability to save the screenshot in the global clipboard, so one can paste it in a VM of choice after grabbing the screenshot.

CumpsD commented 10 months ago

If it helps, I made this:

$ cat qvm-copy-file-to-clipboard 
#!/usr/bin/bash
set -e -o pipefail
#
# The Qubes OS Project, http://www.qubes-os.org
#
# Copyright (C) 2015  Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
#
#

if [ $# -lt 1 ] ; then
    echo usage: $0 'file'
    exit 1
fi

#echo "base64 \"$@\" > /var/run/qubes/qubes-clipboard.bin"
base64 "$@" > /var/run/qubes/qubes-clipboard.bin

#echo "echo -n dom0 > /var/run/qubes/qubes-clipboard.bin.source"
echo -n dom0 > /var/run/qubes/qubes-clipboard.bin.source

if [ "${0##*/}" = "qvm-copy-file-to-clipboard" ]; then
    rm -rf -- "$@"
fi

I then configured this as a custom action in the built in dom0 screenshot tool:

image

image

After I take a screenshot, it copies the file as base64 in the inter-qube clipboard, which I can then CTRL+SHIFT+V into my target qube and run this to get the file in my clipboard to CTRL+V it into github/slack/....

xclip -o | base64 -d | xclip -i -selection clipboard -t image/png

I need to come up with a nice way to trigger this last line in my target VM after pressing CTRL+SHIFT+V, maybe bind the script to yet another keybind?


edit:

I added the following to dom0:

$ cat qvm-parse-base64-clipboard 
#!/bin/bash

# Script should be located at /usr/bin/qvm-parse-base64-clipboard

CUR_WIN_ID=`xdotool getwindowfocus`
CUR_VM=`xprop _QUBES_VMNAME -id $CUR_WIN_ID | cut -d \" -f 2`

if [[ "$CUR_VM" != "_QUBES_VMNAME:  not found." ]]; then
  qvm-run $CUR_VM 'xclip -o | base64 -d | xclip -i -selection clipboard -t image/png'
  notify-send "Screenshot saved!" "Your screenshot is available in the clipboard of $CUR_VM"
fi

and bound CTRL+SHIFT+S to it in the global keyboard.

So now I grab a screenshot, it goes on the global clipboard, I select my VM and press CTRL+SHIFT+V to get it in the qube and CTRL+SHIFT+S to transform it into a screenshot file to paste in slack :)

tlaurion commented 7 months ago

Referred upstream to work done by @ben-grande for reuostreaming and the downstream under contrib at https://github.com/evadogstar/qvm-screenshot-tool/issues/10

Discussion leading to implemtstiom changes prior of qusal inclusion happened at https://github.com/ben-grande/qusal/issues/22

This is a cleaner version of the script, passing through qvm-copy and removing dom0 file edition, limiting the scope to full-screen and active window screenshoting, also removing uploading option which is delegated to be done on the qube where those edit/uploading actions should be done IMO.

As current state, qvm-screenshot per qusal deploys it correctly and works out if the box through qusal. Unfortunately, setting shortcuts in KDE is not so straightforward as it is for xfce, and qusal deploys KDE per deployment of dom0 salt application.

In current state, this means switching from xfce/KDE when needs come to taking screenshot, which to be honest makes sense, since xfce is what is expected to be used by default. So screenshot helper for xfce showing full-screen UX is what people would expect in full-screen screenshot anyway.

Now the question is how to upstream those changes so they make their way to contrib repo.

For commits and accountability I'm tagging @ben-grande here.

I guess the improvements of the scripts should land as a PR against https://github.com/QubesOS-contrib/qubes-qvm-screenshot-tool per contrib guidelines so this ticket can be closed and improvements issues can be opened.

qvm-screenshot is a contrib project under qubes-qvm-screenshot-tool, and contrib guidelines encourage PR against already existing contrib repositories.

ben-grande commented 7 months ago

Referred upstream to work done by @ben-grande for reuostreaming and the downstream under contrib at evadogstar/qvm-screenshot-tool#10

Discussion leading to implemtstiom changes prior of qusal inclusion happened at ben-grande/qusal#22

This is a cleaner version of the script, passing through qvm-copy and removing dom0 file edition, limiting the scope to full-screen and active window screenshoting, also removing uploading option which is delegated to be done on the qube where those edit/uploading actions should be done IMO.

Changes can also be seen in the commit message https://github.com/ben-grande/qusal/commit/134a26a0f50104355d735649ee9cce1d0c875c3a.

As current state, qvm-screenshot per qusal deploys it correctly and works out if the box through qusal. Unfortunately, setting shortcuts in KDE is not so straightforward as it is for xfce, and qusal deploys KDE per deployment of dom0 salt application.

In current state, this means switching from xfce/KDE when needs come to taking screenshot, which to be honest makes sense, since xfce is what is expected to be used by default. So screenshot helper for xfce showing full-screen UX is what people would expect in full-screen screenshot anyway.

You don't need to change from KDE to Xfce, you just need to configure the shortcuts manually for the time being.

Now the question is how to upstream those changes so they make their way to contrib repo.

For commits and accountability I'm tagging @ben-grande here.

I guess the improvements of the scripts should land as a PR against https://github.com/QubesOS-contrib/qubes-qvm-screenshot-tool per contrib guidelines so this ticket can be closed and improvements issues can be opened.

qvm-screenshot is a contrib project under qubes-qvm-screenshot-tool, and contrib guidelines encourage PR against already existing contrib repositories.https://github.com/QubesOS/qubes-issues/issues/953#issuecomment-735701421

About contributing upstream, the repos QubesOS-contrib/qubes-qvm-screenshot-tool and evadogstar/qvm-screenshot-tool differ since 2020. My version also removes a Imgurl, a tool which original author @evadogstar finds important. I don't know how many people it would affect if Imgurl was removed. Although all projects have the same goal of taking screenshot and moving to qubes, they differ in features. I did clean the code extensively more than add features.

I committing enhancements to QubesOS-Contrib is possible, but then it would differ from upstream evadogstar repo. I committing to evadogstar, but they don't open PRs to QubesOS-Contrib for new changes. There is no clear path to upstream it.

marmarek commented 7 months ago

Since https://github.com/QubesOS/qubes-issues/issues/7739 is implemented, we have a way to define per-package maintainers now. So, I have a proposal to improve the screenshoting situation:

  1. Move https://github.com/QubesOS-contrib/qubes-qvm-screenshot-tool to the main organization (and also to the main repository)
  2. Add @ben-grande and/or @evadogstar (if you agree) as the maintainer of the package.
  3. Configure builder to require 2 signed tags on this package - with the idea primary maintainer cares for the functionality and general code shape, and the other person is only verifying if no backdoors or obvious security issues are added (such checking of incremental changes shouldn't be much work given the size of this tool). The second tag can be from the other maintainer, or from qubes core team (for example @fepitre or @DemiMarie or me or ...). To be clear, the general direction and code shape of the tool would be up to the primary maintainer, I do not want nitpicking like "this could be done a bit faster another way" blocking that second tag.

There will need to be a bit coordination needed for which we don't have tooling yet, but I think we can manage even manually for now. Specifically: how to coordinate between those 2 people pushing tags. Ideally, we'd have some system where a PR(?) is merged only when two tags are prepared to be pushed - and then PR merge and two tags are pushed to the repo at the same time. But, in the meantime, we can simply have few people with push access to the repo, and have them coordinate via comments/mail/IM etc. This means at some times (before the second tag is pushed) builder will refuse to fetch the sources, and maybe at times some revert will be needed (when second reviewer spot some issue), but that's IMO still good enough for now.

What do you think about this proposal?

ben-grande commented 7 months ago
  1. Move https://github.com/QubesOS-contrib/qubes-qvm-screenshot-tool to the main organization (and also to the main repository)

I also think it makes sense. It is just some lines of code and leaving it on QubesOS-Contrib make people wait for the original author to make changes while transferring it to the main organization is easier to know where to contribute to (directly).

  1. Add @ben-grande and/or @evadogstar (if you agree) as the maintainer of the package.

I am not sure if if you agree was intended for both people or just the name close to the phrase, therefore I'm answering yes.

  1. Configure builder to require 2 signed tags on this package - with the idea primary maintainer cares for the functionality and general code shape, and the other person is only verifying if no backdoors or obvious security issues are added (such checking of incremental changes shouldn't be much work given the size of this tool). The second tag can be from the other maintainer, or from qubes core team (for example @fepitre or @DemiMarie or me or ...). To be clear, the general direction and code shape of the tool would be up to the primary maintainer, I do not want nitpicking like "this could be done a bit faster another way" blocking that second tag.

Multiple taggers that have at least someone not from ITL would be a nice experiment. I don't want to expand this too much on the screenshot related thread, but lets consider there are at least 3 authorized keys, requiring 2 keys to be valid and 2 of the keys are not used by ITL: me, @fepitre and @evadogstar. Considering ITL keys to be always trustworthy, at least 1 signature needs to come from ITL, else 2 non-ITL keys could sign a malicious commit.

@marmarek suggested this on #7739.

I don't exclude more complex schemes (3 tags from any of A,B,C,D,E or 2 tags if at least one is made by X), but lets start with a simpler option first.

Which doesn't seem to have been completed as far as I know: if at least one is made by X.

There will need to be a bit coordination needed for which we don't have tooling yet, but I think we can manage even manually for now. Specifically: how to coordinate between those 2 people pushing tags. Ideally, we'd have some system where a PR(?) is merged only when two tags are prepared to be pushed - and then PR merge and two tags are pushed to the repo at the same time. But, in the meantime, we can simply have few people with push access to the repo, and have them coordinate via comments/mail/IM etc. This means at some times (before the second tag is pushed) builder will refuse to fetch the sources, and maybe at times some revert will be needed (when second reviewer spot some issue), but that's IMO still good enough for now.

What do you think about this proposal?

Coordination via comments on issues and PR is my preferred method of contact if I become maintainer.

ben-grande commented 2 months ago

Ping.

ben-grande commented 2 months ago

Maintainer @evadogstar has account deleted, the package is "orphaned".

The repo QubesOS-contrib/qubes-qvm-screenshot-tool has lost it's upstream and community maintainer.

The repo evadogstar/qvm-screenshot-tool had some issues opened by the Qubes Core Team at the time, such as @fepitre (to sync commits with Qubes) and @ninavizz (UX), which could be useful improvements ( I don't remember fully their contents)..... how to get that back now that upstream has deleted it's account?


Now on the policy side of opening issues to community owned repos, those things can happen (deletion)... if they were open in qubes-issues, at least a proper backup could have been made.

fepitre commented 2 months ago

I think we can just either drop this package in favor to another one or consider it as the upstream. I don't think we specially need to move it to QubesOS directly. I'm fine to add you as maintainer and we would need to have at least me to add another tag. This is similar to https://github.com/QubesOS-contrib/qubes-app-split-browser where I'm reviewing most of the work done in PR and the maintainer is already preparing everything, I just need to add a tag on it for triggering the build process.

ben-grande commented 2 months ago

Fork made: https://github.com/ben-grande/qubes-qvm-screenshot-tool