Closed
Bug 673586
Opened 12 years ago
Closed 12 years ago
Implement lastModifiedDate property for the File interface
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: sonny, Assigned: dougt)
References
()
Details
(Keywords: dev-doc-complete)
Attachments
(1 file)
4.43 KB,
patch
|
mounir
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux i686; rv:5.0) Gecko/20100101 Firefox/5.0 Build ID: 20110619063910 Expected results: According to http://www.w3.org/TR/FileAPI/#dfn-file the file interface should have a lastModifiedDate property.
Updated•12 years ago
|
OS: Other → All
Updated•12 years ago
|
Keywords: dev-doc-needed
Comment 1•12 years ago
|
||
FWIW this crashes sample file upload code that works just fine w/Chrome; would be nice to have this working! :)
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → doug.turner
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #625670 -
Flags: review?(mounir)
Comment 3•12 years ago
|
||
Comment on attachment 625670 [details] [diff] [review] patch v.1 Review of attachment 625670 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the comments applied. ::: content/base/src/nsDOMFile.cpp @@ +130,5 @@ > > NS_IMETHODIMP > +nsDOMFileBase::GetLastModifiedDate(JSContext* cx, JS::Value *aLastModifiedDate) > +{ > + *aLastModifiedDate = JSVAL_NULL; Please do this instead: aLastModifiedDate->setNull(); @@ +131,5 @@ > NS_IMETHODIMP > +nsDOMFileBase::GetLastModifiedDate(JSContext* cx, JS::Value *aLastModifiedDate) > +{ > + *aLastModifiedDate = JSVAL_NULL; > + return NS_OK; I would whether throw NS_ERROR_NOT_IMPLEMENTED or call NS_ASSERTION. Or both even ;) @@ +427,5 @@ > NS_IMETHODIMP > +nsDOMFileFile::GetLastModifiedDate(JSContext* cx, JS::Value *aLastModifiedDate) > +{ > + PRTime msecs; > + mFile->GetLastModifiedTime(&msecs); AFAICT, this is returning a PRInt64 not PRTime. I know PRTime is typedef to PRInt64 but still... @@ +430,5 @@ > + PRTime msecs; > + mFile->GetLastModifiedTime(&msecs); > + JSObject* date = JS_NewDateObjectMsec(cx, msecs); > + if (date) { > + *aLastModifiedDate = OBJECT_TO_JSVAL(date); aLastModifiedDate->setObject(date); @@ +433,5 @@ > + if (date) { > + *aLastModifiedDate = OBJECT_TO_JSVAL(date); > + } > + else { > + *aLastModifiedDate = JSVAL_NULL; aLastModifieddate->setNull(); ::: content/base/test/test_bug403852.html @@ +33,5 @@ > var domFile = fileList.files[0]; > > is(domFile.name, "prefs.js", "fileName should be prefs.js"); > > +ok(domFile.lastModifiedDate, "lastModifiedDate must be present"); ok("lastModifiedDate" in domFile, "...") would be better. @@ +34,5 @@ > > is(domFile.name, "prefs.js", "fileName should be prefs.js"); > > +ok(domFile.lastModifiedDate, "lastModifiedDate must be present"); > +var d = new Date(testFile.lastModifiedTime); nit: can you move that line with the block below.
Attachment #625670 -
Flags: review?(mounir) → review+
Assignee | ||
Comment 4•12 years ago
|
||
> this is returning a PRInt64 not PRTime. I know PRTime is typedef to PRInt64 but still... They are typed the same per docs. > I would whether throw NS_ERROR_NOT_IMPLEMENTED or call NS_ASSERTION. Or both even ;) I am not sure what we should do here. Maybe ask Jonas (the editor?) and followup. https://hg.mozilla.org/integration/mozilla-inbound/rev/877e541590c2
Assignee | ||
Comment 5•12 years ago
|
||
PRTime followup: https://hg.mozilla.org/integration/mozilla-inbound/rev/09b1b0fe568a
Comment 6•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/877e541590c2 https://hg.mozilla.org/mozilla-central/rev/09b1b0fe568a
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
Comment 7•12 years ago
|
||
Comment on attachment 625670 [details] [diff] [review] patch v.1 Review of attachment 625670 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/src/nsDOMFile.cpp @@ +434,5 @@ > + *aLastModifiedDate = OBJECT_TO_JSVAL(date); > + } > + else { > + *aLastModifiedDate = JSVAL_NULL; > + } Actually, aLastModifieddate->setObjectOrNull(date);
https://developer.mozilla.org/en-US/docs/Firefox_15_for_developers https://developer.mozilla.org/en-US/docs/DOM/File https://developer.mozilla.org/en-US/docs/DOM/File.lastModifiedDate
Keywords: dev-doc-needed → dev-doc-complete
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•