After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 411050 - With some window name, the screenshot tool will propose to replace ~/Desktop with the image
With some window name, the screenshot tool will propose to replace ~/Desktop ...
Status: RESOLVED FIXED
Product: gnome-utils
Classification: Deprecated
Component: screenshot
2.17.x
Other Linux
: Normal major
: ---
Assigned To: Emmanuele Bassi (:ebassi)
gnome-utils Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-02-23 01:50 UTC by Pascal Terjan
Modified: 2007-03-10 17:41 UTC
See Also:
GNOME target: ---
GNOME version: 2.17/2.18


Attachments
The main window, showing correct paths (29.20 KB, image/png)
2007-02-23 01:51 UTC, Pascal Terjan
  Details
The dialog asking for replacement (9.43 KB, image/png)
2007-02-23 01:52 UTC, Pascal Terjan
  Details
strace output (753.78 KB, text/x-log)
2007-02-23 01:53 UTC, Pascal Terjan
  Details
Simple page to reproduce the issue (59 bytes, text/html)
2007-03-04 20:54 UTC, Pascal Terjan
  Details
patch warning of the conversion error (965 bytes, patch)
2007-03-04 22:38 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
real patch (1.84 KB, patch)
2007-03-04 22:50 UTC, Emmanuele Bassi (:ebassi)
none Details | Review

Description Pascal Terjan 2007-02-23 01:50:21 UTC
last.fm has a window called "Recommended Events â~@~S Last.fm" (actually it's a large dash).
When I screenshot it, the proposed filename is OK, and the selected directory is Desktop.
I accept the default settings and then I get a message telling that /home/pterjan/Desktop already exists and asking if I want to replace it. If I say yes, the directory ~/Desktop is removed with all its content, and the image is saved as ~/Desktop.
Comment 1 Pascal Terjan 2007-02-23 01:51:31 UTC
Created attachment 83143 [details]
The main window, showing correct paths
Comment 2 Pascal Terjan 2007-02-23 01:52:06 UTC
Created attachment 83144 [details]
The dialog asking for replacement
Comment 3 Pascal Terjan 2007-02-23 01:53:30 UTC
Created attachment 83145 [details]
strace output
Comment 4 Pascal Terjan 2007-03-04 20:54:34 UTC
Created attachment 83922 [details]
Simple page to reproduce the issue
Comment 5 Emmanuele Bassi (:ebassi) 2007-03-04 21:38:28 UTC
I can't reproduce with trunk (2.17.92).  if you don't see the large dash it means that something in the encoding of your locale is wrong, in my opinion.

anyway, I'll keep trying to reproduce.
Comment 6 Pascal Terjan 2007-03-04 21:58:11 UTC
I see the dash correctly in the page, the dialog..., the strange string was from strace output.
Comment 7 Pascal Terjan 2007-03-04 22:08:08 UTC
Hmm actually I think I can see the origin of the problem.
My system is not using UTF8 (G_FILENAME_ENCODING is @locale, meaning ISO-8859-15) and maybe this character does not exist in this encoding.
But that's strange then that I get no warning from g_filename_from_utf8 or other about failed conversion, and that it then drops the filename and tries to access the directory as a file.
Comment 8 Pascal Terjan 2007-03-04 22:24:51 UTC
OK, I added some printf :

screenshot_dialog_get_uri: folder=file:///home/pterjan/Desktop
screenshot_dialog_get_uri: file_name=Capture-fooâbar.png
screenshot_dialog_get_uri: filename_from_utf8=(null)
screenshot_dialog_get_uri: escaped=(null)
screenshot_dialog_get_uri: uri=file:///home/pterjan/Desktop
run_dialog: uri=file:///home/pterjan/Desktop
try_to_save: target=file:///home/pterjan/Desktop

So, the issue is that filename_from_utf on this string returns NULL, which is not checked.
Comment 9 Emmanuele Bassi (:ebassi) 2007-03-04 22:37:01 UTC
it seems that g_filename_from_utf8() fails to convert the content of the filename entry; this will be fixed by using a GtkFileChooser to select the filename instead of using a separate widget. but, anyway, fixing this bug for the 2.18 cycle would be a good thing.

can you try to apply the following patch and see what kind of error it gets?  the patch also falls back to the default filename ('Screenshot.png') in case of error, so it should eat your directories anymore.
Comment 10 Emmanuele Bassi (:ebassi) 2007-03-04 22:38:10 UTC
Created attachment 83929 [details] [review]
patch warning of the conversion error

this patch applies to gnome-utils/gnome-screenshot trunk, but should work also for the 2.16.x code.
Comment 11 Emmanuele Bassi (:ebassi) 2007-03-04 22:38:55 UTC
obviously, 'should not eat your directories anymore'. :-)
Comment 12 Emmanuele Bassi (:ebassi) 2007-03-04 22:50:58 UTC
Created attachment 83930 [details] [review]
real patch

gaah, should smoketest patches before asking people to test them.

here's the real patch.
Comment 13 Pascal Terjan 2007-03-04 22:52:09 UTC
After adding a declaration for error on the first patch, it looks fine except that it did not translate the default filename. I was offered to overwrite Screenshot.ong instead of Capture.png, but at least it was not the whole directory :)

The warning when run in English :

** (gnome-screenshot:9848): WARNING **: Unable to convert `Screenshot-foo?bar.png' to valid UTF-8: Invalid byte sequence in conversion input
Falling back to default file.
Comment 14 Emmanuele Bassi (:ebassi) 2007-03-04 23:02:07 UTC
yeah, the filename is translated fully ('Screenshot.png') instead of just the filename without the extension as I though. I'll fix it before committing the patch.
Comment 15 Emmanuele Bassi (:ebassi) 2007-03-10 17:41:37 UTC
after 2 approvals from the release-team to break the hard code freeze, the patch has been committed and will appear in release 2.18.0.  thanks again Pascal.

2007-03-10  Emmanuele Bassi  <ebassi@gnome.org>

        * screenshot-dialog.c: Warn (in console) if g_filename_from_utf8()
        fails to convert the screenshot file name in case we are not in
        a utf8 locale, and fall back to the default screenshot file name,
        to avoid eating the user's folders. (#411050, thanks to
        Pascal Terjan for his detective work)