Closed
Bug 958354
Opened 11 years ago
Closed 11 years ago
OS.Constants.Path should expose UAppData
Categories
(Toolkit Graveyard :: OS.File, defect)
Toolkit Graveyard
OS.File
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla29
People
(Reporter: gps, Assigned: sumit4iit)
References
Details
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
AFAICT OS.Constants.Path doesn't expose UAppData (XRE_USER_APP_DATA_DIR). This is preventing porting some nsIFile code to use OS.Path.
Comment 1•11 years ago
|
||
This should be pretty easy.
The file to modify is dom/system/OSFileConstants.cpp. The code will be very similar to the existing code that defines OS.Constants.Path.desktopDir from key NS_OS_DESKTOP_DIR, except we would define OS.Constants.Path.userApplicationDataDir from key XRE_USER_APP_DATA_DIR.
Whiteboard: [Async][mentor=Yoric][lang=c++][good first bug]
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → sumit4iit
Assignee | ||
Comment 2•11 years ago
|
||
Is this the correct way?
Attachment #8358607 -
Flags: feedback?(dteller)
Assignee | ||
Comment 3•11 years ago
|
||
Attachment #8358607 -
Attachment is obsolete: true
Attachment #8358607 -
Flags: feedback?(dteller)
Attachment #8358614 -
Flags: feedback?(dteller)
Comment 4•11 years ago
|
||
Comment on attachment 8358614 [details] [diff] [review]
958354
Review of attachment 8358614 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good, thanks.
::: dom/system/OSFileConstants.cpp
@@ +91,5 @@
> * The user's desktop directory, if there is one. Otherwise this is
> * the same as homeDir.
> */
> nsString desktopDir;
> + nsString userApplicationDataDir;
Documentation would be nice.
Attachment #8358614 -
Flags: feedback?(dteller) → feedback+
Assignee | ||
Comment 5•11 years ago
|
||
Added documentation.
Attachment #8358614 -
Attachment is obsolete: true
Attachment #8358784 -
Flags: review?(dteller)
Comment 6•11 years ago
|
||
Comment on attachment 8358784 [details] [diff] [review]
958354
Review of attachment 8358784 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good.
Before r+-ing, I'd like a test.
Could you add one here? http://dxr.mozilla.org/mozilla-central/source/toolkit/components/osfile/tests/xpcshell/test_path_constants.js
::: dom/system/OSFileConstants.cpp
@@ +103,5 @@
> + * UAppData = $HOME/.[$vendor/]$name
> + *
> + * Mac:
> + * HOME = ~
> + * UAppData = $HOME/Library/Application Support/$name
I suspect that it doesn't exist under Android. Is there a way you could check?
@@ +875,5 @@
> return false;
> }
>
> + if (!SetStringProperty(cx, objPath, "userApplicationDataDir", gPaths->userApplicationDataDir)) {
> + return false;
Nit: Too many spaces before the |return false|.
Attachment #8358784 -
Flags: review?(dteller) → feedback+
Assignee | ||
Comment 7•11 years ago
|
||
while going through code I realized that in
http://dxr.mozilla.org/mozilla-central/source/dom/system/OSFileConstants.cpp#221
|"XpcomLib"| is hard coded. Should I change it to | NS_XPCOM_LIBRARY_FILE | as defined in http://dxr.mozilla.org/mozilla-central/source/xpcom/io/nsDirectoryServiceDefs.h#46 ? Is it ok to do this sort of corrections, or do we need to file new bug for that?
Flags: needinfo?(dteller)
Assignee | ||
Comment 8•11 years ago
|
||
Attachment #8358784 -
Attachment is obsolete: true
Attachment #8358837 -
Flags: feedback?(dteller)
Comment 9•11 years ago
|
||
(In reply to Sumit Agrawal[:sumit4iit] from comment #7)
> while going through code I realized that in
> http://dxr.mozilla.org/mozilla-central/source/dom/system/OSFileConstants.
> cpp#221
> |"XpcomLib"| is hard coded. Should I change it to | NS_XPCOM_LIBRARY_FILE |
> as defined in
> http://dxr.mozilla.org/mozilla-central/source/xpcom/io/
> nsDirectoryServiceDefs.h#46 ? Is it ok to do this sort of corrections, or do
> we need to file new bug for that?
You can do it if you want.
Flags: needinfo?(dteller)
Updated•11 years ago
|
Attachment #8358837 -
Flags: feedback?(dteller) → review+
Assignee | ||
Comment 10•11 years ago
|
||
Attachment #8358837 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Whiteboard: [Async][mentor=Yoric][lang=c++][good first bug]
Comment 11•11 years ago
|
||
Comment 12•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla29
Updated•2 years ago
|
Product: Toolkit → Toolkit Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•