Bug 59527 - uses old gtk-bookmarks location
Summary: uses old gtk-bookmarks location
Status: RESOLVED FIXED
Alias: None
Product: xdg-user-dirs
Classification: Unclassified
Component: General (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Alexander Larsson
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-17 20:22 UTC by william.jon.mccann
Modified: 2013-01-21 18:10 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
part 1 (3.34 KB, patch)
2013-01-17 21:04 UTC, Cosimo Cecchi
Details | Splinter Review
part 2 (2.66 KB, patch)
2013-01-17 21:04 UTC, Cosimo Cecchi
Details | Splinter Review
part 2 (2.69 KB, patch)
2013-01-17 21:06 UTC, Cosimo Cecchi
Details | Splinter Review
part 2 (2.74 KB, patch)
2013-01-21 15:03 UTC, Cosimo Cecchi
Details | Splinter Review

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.