Bug 1752514

I think this is expected. The change came from below bug.

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

So if it is by intention it is not a bug. So going to close this one and modify our testcase.

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

 Modified the code to add a default extension in this use case.

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

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.

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.

(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?

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.

(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.

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

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.

Thanks Julien.
I move this bug from ON_QA to VERIFIED status based on Comment 13 testing.

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