Closed
Bug 399500
Opened 17 years ago
Closed 16 years ago
support XDG user dirs in the directory service
Categories
(Core :: XPCOM, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla1.9beta5
People
(Reporter: chpe, Assigned: chpe)
References
()
Details
Attachments
(1 file, 1 obsolete file)
18.63 KB,
patch
|
caillon
:
review+
benjamin
:
superreview+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
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.
Updated•17 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 1•17 years ago
|
||
Updated•17 years ago
|
Assignee: nobody → chpe
Assignee | ||
Comment 2•17 years ago
|
||
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)
Assignee | ||
Comment 3•17 years ago
|
||
Attachment #284519 -
Attachment is obsolete: true
Attachment #286149 -
Flags: review?(benjamin)
Attachment #284519 -
Flags: review?(benjamin)
Updated•16 years ago
|
Flags: blocking1.9?
Comment 4•16 years ago
|
||
Won't block on this, but if reviewed, ask for approval.
Flags: blocking1.9? → blocking1.9-
Comment 5•16 years ago
|
||
Christian, the patch doesn't apply cleanly anymore. Mind to update it (and also the one on Bug 399498)?
Assignee | ||
Comment 6•16 years ago
|
||
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.
Comment 7•16 years ago
|
||
(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...
Comment 8•16 years ago
|
||
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 9•16 years ago
|
||
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)
Assignee | ||
Comment 10•16 years ago
|
||
(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...
Comment 11•16 years ago
|
||
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 12•16 years ago
|
||
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+
Updated•16 years ago
|
Attachment #286149 -
Flags: superreview?(benjamin)
Updated•16 years ago
|
Attachment #286149 -
Flags: superreview?(benjamin) → superreview+
Comment 14•16 years ago
|
||
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 15•16 years ago
|
||
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+
Updated•16 years ago
|
Keywords: checkin-needed
Comment 16•16 years ago
|
||
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
Comment 17•16 years ago
|
||
gerv, do we need to update about:license to include the license concerning the xdg_user_dir_lookup function?
Comment 18•16 years ago
|
||
Reed: Yes. Please file a bug to that effect. Gerv
Comment 19•16 years ago
|
||
Should I also verify this if I have just verified bug 399498?
You need to log in
before you can comment on or make changes to this bug.
Description
•