Bug 1752514 - Screenshot is not saved if no extension specified
Summary: Screenshot is not saved if no extension specified
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 8
Classification: Red Hat
Component: virt-viewer
Version: 8.1
Hardware: Unspecified
OS: Unspecified
unspecified
low
Target Milestone: rc
: 8.0
Assignee: Julien Ropé
QA Contact: zhoujunqin
URL:
Whiteboard:
Depends On: 1837489
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-09-16 13:52 UTC by Radek Duda
Modified: 2020-11-04 03:53 UTC (History)
7 users (show)

Fixed In Version: virt-viewer-9.0-2.el8
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-11-04 03:53:18 UTC
Type: Bug
Target Upstream Version:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2020:4788 0 None None None 2020-11-04 03:53:42 UTC

Description Radek Duda 2019-09-16 13:52:20 UTC
Description of problem:
Screenshot is not saved if no extension specified

Version-Release number of selected component (if applicable):
virt-viewer-7.0-8.el8.x86_64

How reproducible:
always

Steps to Reproduce:
1. Run virt-viewer and connect to some running VM
2. Mouse click virt-viewer menu: File -> Screenshot
3. try to save as $some_file_name_without_extension

Actual results:
message "Unable to determine image format for file $some_file_name_without_extension" appears

Expected results:
Screenshot should be saved in PNG format.

Additional info:

Comment 1 Xiaodai Wang 2019-09-17 02:39:22 UTC
I think this is expected. The change came from below bug.

Bug 1455832 - No overwrite confirmation pops if keep name as default 'Screenshot'

Comment 2 Radek Duda 2019-09-17 09:20:33 UTC
So if it is by intention it is not a bug. So going to close this one and modify our testcase.

Comment 3 Daniel Berrangé 2019-09-17 09:29:35 UTC
Re-opening, this current behaviour is not desirable IMHO -  we should append .png if no extension is present as we did in the past. We can still do proper overwrite confirmations

Comment 4 Julien Ropé 2020-02-19 13:58:22 UTC
 Modified the code to add a default extension in this use case.

 https://pagure.io/virt-viewer/c/e4bacb8fde16cd21b8b8f095be720ad1a6c2d0e5?branch=master

Comment 7 zhoujunqin 2020-06-08 10:22:03 UTC
I can reproduce with package:
virt-viewer-7.0-9.el8.x86_64
libgovirt-0.3.4-9.el8.x86_64

Steps:
1. Run virt-viewer and connect to some running VM
$ virt-viewer -c qemu:///system rhel7.9
2. Mouse click virt-viewer menu: File -> Screenshot
3. Try to save as $some_file_name_without_extension(eg: Screenshot)

Result: Failed to save with error message - "Unable to determine image format for file $some_file_name_without_extension" appears

Then try to verify with new build:
virt-viewer-9.0-1.el8.x86_64
libgovirt-0.3.7-1.el8.x86_64

Steps:
1. Run virt-viewer and connect to some running VM
$ virt-viewer -c qemu:///system rhel7.9
2. Mouse click virt-viewer menu: File -> Screenshot
3. Try to save as $some_file_name_without_extension(eg: Screenshot)
4. Repeat step3.

Result after step3:
i. no error message appears when I click 'Save'
ii. Check the screenshot just saved in folder: 
[juzhou@localhost Pictures]$ ll
total 448
-rw-rw-r--. 1 juzhou juzhou 301221 Jun  8 17:54 Screenshot.png


It seems that.png is appended when no extension set, it's as expected.

Result after step4:
No warning message prompts when saving the screenshot with the same name 'Screen', and the file is overwritten by default.

$ ll
total 448
-rw-rw-r--. 1 juzhou juzhou 196857 Jun  8 17:55 Screenshot.png

But when i set the saved screenshot name as '1.png' twice, there is a warning message as usual:
"""

A file named "1.png" already exists. Do you want
to replace it?

The file already exists in "Pictures". Replace it will overwrite its contents.

"""

Make a summary of my testing result:
1).png is appended when no extension set and the screenshot will be saved successfully.
2) no warning message for step4, it should be fixed i hope.


@Julien Ropé and @Eduardo Lima, the bug issue has been fixed based on result for step3, please help me have check the result for step4, thanks.

Comment 8 zhoujunqin 2020-06-08 11:10:11 UTC
And one more test scenario for Comment 7:

2.1 Run virt-viewer and connect to some running VM
$ virt-viewer -c qemu:///system rhel7.9
2.2 Mouse click virt-viewer menu: File -> Screenshot
2.3 Try to save the screenshot as name "a.png"
2.4 Repeat step2.1 to 2.3, save the screenshot as name "a".

Result:
2.3 Screenshot with name "a.png" was saved successfully.

$ ll
total 448
-rw-rw-r--. 1 juzhou juzhou 554737 Jun  8 18:49 a.png

2.4 Screenshot was saved, and .png is appended after "a", check the new screenshot.
$ ll
total 448
-rw-rw-r--. 1 juzhou juzhou 565259 Jun  8 19:03 a.png

The previous 'a.png' is overwritten silently. 

As a sumary, if above issue can't be fixed, how about add a warning to let user know if you save a screenshot without extension set, the previous file with same name may be overwritten, or we can record this situation in man page, thanks.

Comment 9 Julien Ropé 2020-06-08 15:50:26 UTC
(In reply to zhoujunqin from comment #8)

Thanks for finding this.
This is something we need to address, but maybe not as part of this bug - do you mind opening a new bug for it so that we deal with it properly?

Comment 10 Julien Ropé 2020-06-08 16:21:01 UTC
Sorry, please ignore my last comment.
Discussing with the team, we may have a better solution to do it.

I will try to do another fix ASAP.

Comment 11 zhoujunqin 2020-06-09 01:50:24 UTC
(In reply to Julien Ropé from comment #10)
> Sorry, please ignore my last comment.
> Discussing with the team, we may have a better solution to do it.
> 
> I will try to do another fix ASAP.

Got it, thanks.
So I change the bug status from ON_QA to ASSIGNED, thanks.

Comment 12 Julien Ropé 2020-06-09 14:44:23 UTC
I have made another patch, which shows an error message and let the user enter filename with an extension.
This ensure the check for an existing file is made properly.

See commit https://gitlab.com/virt-viewer/virt-viewer/-/commit/c6afc28cc9761af3e992eab0ca105a978b83a346

Comment 13 zhoujunqin 2020-06-11 10:14:43 UTC
Retest the bug with latest virt-viewer version:

virt-viewer-9.0-2.el8.x86_64

Steps:
1. Run virt-viewer and connect to some running VM
$ virt-viewer -c qemu:///system rhel7.9

2. Mouse click virt-viewer menu: File -> Screenshot

3. Try to save as $some_file_name_without_extension(eg: a)

Result:
Failed to save the screenshot with message "Please add an extension to the file name" appears, then click 'Close' button and nevigate back to the Name field.

4. Try to save as $some_file_name_with_extension(eg:a.png)

Result: Screenshot is saved successfully.
$ ll
total 8
-rw-rw-r--. 1 juzhou juzhou 5191 Jun 11 16:51 a.png

5. Repeat step3 again

Result: Get the same result as step3.

6. Repeat step4 again

Result: A warning message appears:
"""

A file named "a.png" already exists. Do you want
to replace it?

The file already exists in "Pictures". Replace it will overwrite its contents.

"""

Click "Cancel" ---> Nevigate back to filling the Name field.
Click "Replace" ---> The saved screenshot is replaced successfully.
$ ll
total 448
-rw-rw-r--. 1 juzhou juzhou 411368 Jun 11 17:06 a.png

7. Try to save screenshot with invalid extension format.(eg: a.)

Result: A message appears: Unable to determine image format for file '/home/juzhou/Pictures/a'.

As a summary, all results are as expected, thanks for your fix.

@Julien Ropé, could you help move this bug to ON_QA status, then I can move it to VERIFIED, thanks for your time.

Comment 14 zhoujunqin 2020-06-11 13:13:15 UTC
Thanks Julien.
I move this bug from ON_QA to VERIFIED status based on Comment 13 testing.

Comment 17 errata-xmlrpc 2020-11-04 03:53:18 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (virt-viewer bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:4788


Note You need to log in before you can comment on or make changes to this bug.