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 750498 - "Desktop" shortcut appears twice in file chooser sidebar on OSX
"Desktop" shortcut appears twice in file chooser sidebar on OSX
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: general
unspecified
Other Mac OS
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2015-06-06 19:31 UTC by draymond
Modified: 2018-05-24 17:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
file chooser dialog on OSX (146.43 KB, image/png)
2015-06-06 19:31 UTC, draymond
  Details
Don't return a Downloads folder on OS X (1.20 KB, patch)
2015-06-08 13:32 UTC, Matthias Clasen
none Details | Review
Fix special user dir lookup on OSX (3.76 KB, patch)
2015-06-08 22:48 UTC, Patrick Griffis (tingping)
none Details | Review
Fix special user dir lookup on OSX (3.79 KB, patch)
2015-06-08 22:54 UTC, Patrick Griffis (tingping)
reviewed Details | Review
backtrace of segfault when closing file chooser dialog (1.69 KB, text/plain)
2015-06-11 00:19 UTC, draymond
  Details

Description draymond 2015-06-06 19:31:35 UTC
Created attachment 304713 [details]
file chooser dialog on OSX

There are two "Desktop" shortcuts in the file chooser sidebar.  They have different icons.  When the user clicks the lower one it jumps to the upper one (see screenshot).
Comment 1 Matthias Clasen 2015-06-08 12:05:26 UTC
This looks like you have XDG_DOWNLOADS_DIR configured to be '$HOME/Desktop' - the icon is the one we use for Downloads.
Comment 2 Matthias Clasen 2015-06-08 12:08:10 UTC
Actually, it comes from this:

static void
load_user_special_dirs (void)
{
  g_user_special_dirs[G_USER_DIRECTORY_DESKTOP] = find_folder (kDesktopFolderType);
  g_user_special_dirs[G_USER_DIRECTORY_DOCUMENTS] = find_folder (kDocumentsFolderType);
  g_user_special_dirs[G_USER_DIRECTORY_DOWNLOAD] = find_folder (kDesktopFolderType); /* XXX correct ? */
  g_user_special_dirs[G_USER_DIRECTORY_MUSIC] = find_folder (kMusicDocumentsFolderType);
  g_user_special_dirs[G_USER_DIRECTORY_PICTURES] = find_folder (kPictureDocumentsFolderType);
  g_user_special_dirs[G_USER_DIRECTORY_PUBLIC_SHARE] = NULL;
  g_user_special_dirs[G_USER_DIRECTORY_TEMPLATES] = NULL;
  g_user_special_dirs[G_USER_DIRECTORY_VIDEOS] = find_folder (kMovieDocumentsFolderType);
}
Comment 3 Matthias Clasen 2015-06-08 12:17:36 UTC
We should probably just change that to NULL instead of returning a duplicate.
Comment 4 Matthias Clasen 2015-06-08 13:32:57 UTC
Created attachment 304770 [details] [review]
Don't return a Downloads folder on OS X

OS X doesn't seem to have a dedicated location for this purpose,
and returning Desktop for it leads to duplicates in the filechooser.

http://bugzilla.gnome.org/show_bug.cgi?id=750498
Comment 5 Emmanuele Bassi (:ebassi) 2015-06-08 14:28:06 UTC
I'm not entirely sure; since 10.6, there *is* a Downloads directory.

The main issue is that, starting from 10.10, the whole low-level enumeration has been dropped, which makes the C fallback only useful for versions of MacOS X < 10.6. Starting from 10.6, we should switch to using NSFileManager:

- (NSURL *)URLForDirectory:(NSSearchPathDirectory)directory
                  inDomain:(NSSearchPathDomainMask)domain
         appropriateForURL:(NSURL *)url
                    create:(BOOL)shouldCreate
                     error:(NSError **)error

Using the constants from:

https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Miscellaneous/Foundation_Constants/index.html#//apple_ref/doc/c_ref/NSSearchPathDirectory

and the mask bits from:

https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Miscellaneous/Foundation_Constants/index.html#//apple_ref/doc/c_ref/NSSearchPathDomainMask
Comment 6 Matthias Clasen 2015-06-08 15:44:09 UTC
looks promising, but I'm not the right person to write that patch
Comment 7 Patrick Griffis (tingping) 2015-06-08 22:48:47 UTC
Created attachment 304820 [details] [review]
Fix special user dir lookup on OSX
Comment 8 Patrick Griffis (tingping) 2015-06-08 22:54:25 UTC
Created attachment 304821 [details] [review]
Fix special user dir lookup on OSX
Comment 9 Emmanuele Bassi (:ebassi) 2015-06-09 00:06:10 UTC
Review of attachment 304821 [details] [review]:

Generally, it looks okay to me; if we don't support MacOS < 10.6 then this can go in as is. If we want to support MacOS 10.5 for the poor lost souls that still own a PPC Mac and want bleeding edge dependencies nonetheless, we could do a compile-time check and leave the old code in.
Comment 10 Patrick Griffis (tingping) 2015-06-09 00:25:01 UTC
(In reply to Emmanuele Bassi (:ebassi) from comment #9)
> Review of attachment 304821 [details] [review] [review]:
> 
> Generally, it looks okay to me; if we don't support MacOS < 10.6 then this
> can go in as is. If we want to support MacOS 10.5 for the poor lost souls
> that still own a PPC Mac and want bleeding edge dependencies nonetheless, we
> could do a compile-time check and leave the old code in.

Currently HEAD only supports 10.9. I talked to jralls about it and he really wants it to be optional but I don't know what will come of that.
Comment 11 draymond 2015-06-10 05:23:59 UTC
I tried this patch and the Downloads bookmark is working now.  However, I am getting a segfault when exiting the file chooser dialog.  Have you tested it TingPing?
Comment 12 Patrick Griffis (tingping) 2015-06-10 09:01:53 UTC
(In reply to draymond from comment #11)
> I tried this patch and the Downloads bookmark is working now.  However, I am
> getting a segfault when exiting the file chooser dialog.  Have you tested it
> TingPing?

Yea I had no crashes, got a backtrace?
Comment 13 draymond 2015-06-11 00:19:48 UTC
Created attachment 305023 [details]
backtrace of segfault when closing file chooser dialog
Comment 14 draymond 2015-06-11 01:01:32 UTC
I can suppress the crash by commenting out the following lines:

[paths release];

Are supposed to release the pointer returned by NSSearchPathForDirectoriesInDomains()?
Comment 15 Patrick Griffis (tingping) 2015-06-11 02:32:45 UTC
(In reply to draymond from comment #14)
> I can suppress the crash by commenting out the following lines:
> 
> [paths release];
> 
> Are supposed to release the pointer returned by
> NSSearchPathForDirectoriesInDomains()?

Hmm, it is possible not, I find apples docs extremely unclear about ownership.
Comment 16 draymond 2015-06-12 18:49:10 UTC
From what I can tell the caller does not own the pointer returned by NSSearchPathForDirectoriesInDomains() and therefore should not free it.  Will this patch be released (after fixing the invalid free operation)?
Comment 17 draymond 2017-01-30 05:09:57 UTC
This issue still exists in 3.22.7.
Comment 18 GNOME Infrastructure Team 2018-05-24 17:53:56 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/1048.