Closed Bug 958354 Opened 11 years ago Closed 11 years ago

OS.Constants.Path should expose UAppData

Categories

(Toolkit Graveyard :: OS.File, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla29

People

(Reporter: gps, Assigned: sumit4iit)

References

Details

Attachments

(1 file, 4 obsolete files)

AFAICT OS.Constants.Path doesn't expose UAppData (XRE_USER_APP_DATA_DIR). This is preventing porting some nsIFile code to use OS.Path.
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: nobody → sumit4iit
Attached patch 958354 (obsolete) (deleted) — Splinter Review
Is this the correct way?
Attachment #8358607 - Flags: feedback?(dteller)
Attached patch 958354 (obsolete) (deleted) — Splinter Review
Attachment #8358607 - Attachment is obsolete: true
Attachment #8358607 - Flags: feedback?(dteller)
Attachment #8358614 - Flags: feedback?(dteller)
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+
Attached patch 958354 (obsolete) (deleted) — Splinter Review
Added documentation.
Attachment #8358614 - Attachment is obsolete: true
Attachment #8358784 - Flags: review?(dteller)
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+
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)
Attached patch added test (obsolete) (deleted) — Splinter Review
Attachment #8358784 - Attachment is obsolete: true
Attachment #8358837 - Flags: feedback?(dteller)
(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)
Attachment #8358837 - Flags: feedback?(dteller) → review+
Attached patch 958354 (deleted) — Splinter Review
Attachment #8358837 - Attachment is obsolete: true
Keywords: checkin-needed
Whiteboard: [Async][mentor=Yoric][lang=c++][good first bug]
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla29
Blocks: 963327
No longer blocks: 1388134
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: