Closed Bug 399500 Opened 17 years ago Closed 16 years ago

support XDG user dirs in the directory service

Categories

(Core :: XPCOM, enhancement)

x86
Linux
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9beta5

People

(Reporter: chpe, Assigned: chpe)

References

()

Details

Attachments

(1 file, 1 obsolete file)

The XDG user dirs (see URL) are used by GNOME to find, and allow the user to change, common directories with localised names. To enable using the Download directory in the toolkit's download manager (bug 399498), the directory service needs to support these directories.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → chpe
Comment on attachment 284519 [details] [diff] [review]
proposed implementation. The included xdg_user_dir_lookup code is provided by freedesktop.org under a BSD-style licence which is compatible with mozilla's tri-licence.

I guess it may be too late for 1.9 for this; but asking doesn't cost anything :)
Attachment #284519 - Flags: review?(benjamin)
Attachment #284519 - Attachment is obsolete: true
Attachment #286149 - Flags: review?(benjamin)
Attachment #284519 - Flags: review?(benjamin)
Flags: blocking1.9?
Won't block on this, but if reviewed, ask for approval.
Flags: blocking1.9? → blocking1.9-
Christian, the patch doesn't apply cleanly anymore. Mind to update it (and also the one on Bug 399498)?
How does it fail? Probably just because surrounding lines have changed. I could re-diff, but I have to edit out unrelated changed in my tree, so it's not a zero-cost operation. I would do it if there was any chance this patch could get review and land in 1.9, but that doesn't seem likely.
(In reply to comment #6)
> How does it fail? Probably just because surrounding lines have changed.

Probably.

> I could re-diff, but I have to edit out unrelated changed in my tree, so it's
> not a zero-cost operation. I would do it if there was any chance this patch
> could get review and land in 1.9, but that doesn't seem likely.

Well, as long as it doesn't apply, this seems even more unlikely... OTOH, if that's too much of a hassle for you, OK then...
It doesn't need to apply for someone to provide comments on the patch.

For example:
chpe, does it really make sense for us to copy the xdg_user_dir_lookup function?  glib 2.14 added g_get_user_special_dir () and I don't believe there are any systems below that which would actually have translated directories, so we can simply return the untranslated ones without the need for the extra code if not on glib 2.14.  Also, do we really need to care about things other than Desktop?
Comment on attachment 286149 [details] [diff] [review]
updated patch, to also provide the NS_UNIX_DEFAULT_DOWNLOAD_DIR key

I'll delegate review to caillon, who seems to know more about it than I.

I do worry that we're adding a bunch of XDG-specific directory keys, when we should just have a single "downloads directory", "desktop directory",  "music directory" key to which each platform responds with the platform download directory.
Attachment #286149 - Flags: review?(benjamin) → review?(caillon)
(In reply to comment #8)
> chpe, does it really make sense for us to copy the xdg_user_dir_lookup
> function?  glib 2.14 added g_get_user_special_dir () and I don't believe there
> are any systems below that which would actually have translated directories, so
> we can simply return the untranslated ones without the need for the extra code
> if not on glib 2.14.  Also, do we really need to care about things other than
> Desktop?

I didn't use the glib functions since I wasn't sure a hard dep on glib >= 2.14 was acceptable, and I dislike the dlsym hackery used otherwise in these cases to load the symbols at runtime... 

(In reply to comment #9)
> (From update of attachment 286149 [details] [diff] [review])
> I do worry that we're adding a bunch of XDG-specific directory keys, when we
> should just have a single "downloads directory", "desktop directory",  "music
> directory" key to which each platform responds with the platform download
> directory.

I see a bunch of OSX specific keys in nsDirectoryServiceDefs.h that are similar, (NS_OSX_PICTURE_DOCUMENTS_DIR, NS_OSX_MOVIE_DOCUMENTS_DIR, NS_OSX_MUSIC_DOCUMENTS_DIR), but we don't seem to have these for windows, so I added them in the NS_UNIX_XDG_ namespace. Also the one I started with this work for, the downloads folder, is defined separately for each major platform too...
Hm, I just marked bug 259356 as dependent on this one, but I'm not sure about it anymore. Feel free to remove it from dependent bugs list if that's not the case.
Comment on attachment 286149 [details] [diff] [review]
updated patch, to also provide the NS_UNIX_DEFAULT_DOWNLOAD_DIR key

r=caillon.  The code is good, but bsmedberg gets the last word on the naming.
Attachment #286149 - Flags: review?(caillon) → review+
Attachment #286149 - Flags: superreview?(benjamin) → superreview+
Comment on attachment 286149 [details] [diff] [review]
updated patch, to also provide the NS_UNIX_DEFAULT_DOWNLOAD_DIR key

This fixes the core backend to allow for parity with Mac and Windows on automatically selecting the proper directory for downloads. Bug 399498 will use this backend code to actually support selecting the proper directory.
Attachment #286149 - Flags: approval1.9?
Comment on attachment 286149 [details] [diff] [review]
updated patch, to also provide the NS_UNIX_DEFAULT_DOWNLOAD_DIR key

a1.9+=damons
Attachment #286149 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Checking in xpcom/io/SpecialSystemDirectory.cpp;
/cvsroot/mozilla/xpcom/io/SpecialSystemDirectory.cpp,v  <--  SpecialSystemDirectory.cpp
new revision: 1.39; previous revision: 1.38
done
Checking in xpcom/io/SpecialSystemDirectory.h;
/cvsroot/mozilla/xpcom/io/SpecialSystemDirectory.h,v  <--  SpecialSystemDirectory.h
new revision: 1.10; previous revision: 1.9
done
Checking in xpcom/io/nsDirectoryService.cpp;
/cvsroot/mozilla/xpcom/io/nsDirectoryService.cpp,v  <--  nsDirectoryService.cpp
new revision: 1.95; previous revision: 1.94
done
Checking in xpcom/io/nsDirectoryService.h;
/cvsroot/mozilla/xpcom/io/nsDirectoryService.h,v  <--  nsDirectoryService.h
new revision: 1.35; previous revision: 1.34
done
Checking in xpcom/io/nsDirectoryServiceDefs.h;
/cvsroot/mozilla/xpcom/io/nsDirectoryServiceDefs.h,v  <--  nsDirectoryServiceDefs.h
new revision: 1.31; previous revision: 1.30
done
Status: NEW → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta5
gerv, do we need to update about:license to include the license concerning the xdg_user_dir_lookup function?
Reed: Yes. Please file a bug to that effect.

Gerv
Blocks: 422692
Should I also verify this if I have just verified bug 399498?
Regressions: 434327
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: