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 679055 - Don't mix file and URI related functions
Don't mix file and URI related functions
Status: RESOLVED FIXED
Product: gnome-screenshot
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-screenshot-maint
gnome-screenshot-maint
Depends on:
Blocks:
 
 
Reported: 2012-06-28 11:38 UTC by Bastien Nocera
Modified: 2012-06-28 16:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Don't mix file and URI related functions (4.81 KB, patch)
2012-06-28 11:38 UTC, Bastien Nocera
reviewed Details | Review
Fix crasher on exit with URI/path changes (1.21 KB, patch)
2012-06-28 11:39 UTC, Bastien Nocera
needs-work Details | Review

Description Bastien Nocera 2012-06-28 11:38:05 UTC
.
Comment 1 Bastien Nocera 2012-06-28 11:38:07 UTC
Created attachment 217508 [details] [review]
Don't mix file and URI related functions

g_strconcat ("file://", ...); is never a good way to create URIs
and most of the functions worked on local paths instead of URIs, so
use paths everywhere.
Comment 2 Bastien Nocera 2012-06-28 11:39:09 UTC
Created attachment 217509 [details] [review]
Fix crasher on exit with URI/path changes

Now that screenshot_build_filename_async() operates on path,
it will also return a path. Make sure the caller can handle that.
Comment 3 Cosimo Cecchi 2012-06-28 15:44:41 UTC
Review of attachment 217508 [details] [review]:

The only thing that doesn't fully convince me here is that AFAICS we lose the ability to save a VFS URI.
In interactive mode, we remember the last location used, and set that as the initial location for the next file chooser...does that work for remote VFS locations after this patch?
Comment 4 Cosimo Cecchi 2012-06-28 15:45:25 UTC
Review of attachment 217509 [details] [review]:

This should be squashed to the previous patch if we end up pushing it.
Comment 5 Bastien Nocera 2012-06-28 15:55:35 UTC
(In reply to comment #3)
> Review of attachment 217508 [details] [review]:
> 
> The only thing that doesn't fully convince me here is that AFAICS we lose the
> ability to save a VFS URI.
> In interactive mode, we remember the last location used, and set that as the
> initial location for the next file chooser...does that work for remote VFS
> locations after this patch?

I wasn't sure whether you wanted that.
In this case, it should be as easy as replacing:
+      retval = g_filename_from_uri (save_dir, NULL, NULL);
with:
GFile *file;
file = g_file_new_for_uri (save_dir);
retval = g_file_get_path (file);
g_object_unref (file);

You're lucky this didn't break before. screenshot_build_filename_finish() is randomly returning URIs or filepaths depending on which path it takes.
Comment 6 Bastien Nocera 2012-06-28 16:24:02 UTC
Merged the 2 patches, and made sure both sections could handle remote URIs, using GFile rather than g_filename_{to,from}_uri()