Bug 59527

Summary: uses old gtk-bookmarks location
Product: xdg-user-dirs Reporter: william.jon.mccann
Component: GeneralAssignee: Alexander Larsson <alexl>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: cosimoc
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: part 1
part 2
part 2
part 2

Description william.jon.mccann 2013-01-17 20:22:13 UTC
Gtk+ 3 has been migrated to use the XDG Basedir spec to store bookmarks. We should update the user dirs to do the same.

http://git.gnome.org/browse/xdg-user-dirs-gtk/tree/parse.c#n169
Comment 1 Cosimo Cecchi 2013-01-17 21:04:27 UTC
Created attachment 73210 [details] [review]
part 1
Comment 2 Cosimo Cecchi 2013-01-17 21:04:42 UTC
Created attachment 73211 [details] [review]
part 2
Comment 3 Cosimo Cecchi 2013-01-17 21:06:26 UTC
Created attachment 73212 [details] [review]
part 2
Comment 4 Matthias Clasen 2013-01-18 02:19:01 UTC
Comment on attachment 73212 [details] [review]
part 2

Review of attachment 73212 [details] [review]:
-----------------------------------------------------------------

::: parse.c
@@ +16,5 @@
> +  legacy_filename = g_build_filename (g_get_home_dir (), ".gtk-bookmarks", NULL);
> +  legacy_exists = g_file_test (legacy_filename, G_FILE_TEST_EXISTS);
> +
> +  if (legacy_exists)
> +    return legacy_filename;

When returning here, you leak filename
Comment 5 Alexander Larsson 2013-01-21 08:10:17 UTC
Is gtk2 also updated to read from the new location? If not it seems that we're not upgrading gtk2 apps and we should also replace any existing old file.
Comment 6 Cosimo Cecchi 2013-01-21 15:01:26 UTC
(In reply to comment #5)
> Is gtk2 also updated to read from the new location? If not it seems that
> we're not upgrading gtk2 apps and we should also replace any existing old
> file.

Yeah, GTK2 is also updated to read from the new location since 2.24.14. I'll post a new patch for the leak found by Matthias.
Comment 7 Cosimo Cecchi 2013-01-21 15:03:11 UTC
Created attachment 73386 [details] [review]
part 2
Comment 8 Matthias Clasen 2013-01-21 18:10:05 UTC
already pushed with the memleak fix

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.