Before 2012 Oct the expected behavior in the previous spec was to return 'null', but the latest spec says it should return a new Date object with the current date/time.
Created attachment 191413 [details] Patch
Comment on attachment 191413 [details] Patch Attachment 191413 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17052026 New failing tests: http/tests/local/fileapi/file-last-modified-after-delete.html
Comment on attachment 191413 [details] Patch Attachment 191413 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17029053 New failing tests: inspector/console/console-eval-global.html
Comment on attachment 191413 [details] Patch Attachment 191413 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17009046 New failing tests: http/tests/local/fileapi/file-last-modified-after-delete.html
Created attachment 191459 [details] Patch
Comment on attachment 191459 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191459&action=review > Source/WebCore/ChangeLog:9 > + File.lastModifiedDate() should return the current date/time if the file date/time is not available > + https://bugs.webkit.org/show_bug.cgi?id=111403 > + > + Per the recent File API spec change: > + http://www.w3.org/TR/2012/WD-FileAPI-20121025/#dfn-lastModifiedDate > + > + Reviewed by NOBODY (OOPS!). Normal ChangeLog format is: <summary> <bug URL> Reviewed by <details> Test: .. > Source/WebCore/fileapi/File.cpp:154 > + time_t modificationTime; You don't need to move the declaration. > Source/WebCore/fileapi/File.cpp:162 > + return modificationTime * 1000.0; nit: Use WTF::msPerSecond instead of 1000.0. > Source/WebCore/fileapi/File.cpp:164 > + return currentTime() * 1000.0; Ditto.
What does it mean for a file to not have a modification time? Is that when there is no read access to its attributes?
(In reply to comment #7) > What does it mean for a file to not have a modification time? Is that when there is no read access to its attributes? I read the spec as the availability of file modification time could be platform- -dependent (like file creation time attribute is not available on Windows). In this patch I'm returning the current date/time when WebCore::getFileModificationTime() (implemented by platform layer) returns false. The exact phrase in the spec is: spec> lastModifiedDate: The last modified date of the file. On getting, if user agents can make this information available, this must return a new Date[HTML] object initialized to the last modified date of the file. If the last modification date and time are not known, the attribute must return the current date and time as a Date object.
Created attachment 191650 [details] Patch
Comment on attachment 191459 [details] Patch Thanks, updated the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=191459&action=review >> Source/WebCore/ChangeLog:9 >> + Reviewed by NOBODY (OOPS!). > > Normal ChangeLog format is: > > <summary> > <bug URL> > > Reviewed by > > <details> > > Test: .. Oh... yes, thanks. Fixed. >> Source/WebCore/fileapi/File.cpp:154 >> + time_t modificationTime; > > You don't need to move the declaration. Fixed. >> Source/WebCore/fileapi/File.cpp:162 >> + return modificationTime * 1000.0; > > nit: Use WTF::msPerSecond instead of 1000.0. It's for micro-secs per milliseconds, so I defined another constant.
Comment on attachment 191459 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191459&action=review >>> Source/WebCore/fileapi/File.cpp:162 >>> + return modificationTime * 1000.0; >> >> nit: Use WTF::msPerSecond instead of 1000.0. > > It's for micro-secs per milliseconds, so I defined another constant. Really? I think time_t represents seconds in many platforms, and our Date binding code assume a double value as milliseconds.
Created attachment 191696 [details] Patch
Comment on attachment 191459 [details] Patch Updated the patch again. Can you take another look? Thanks! View in context: https://bugs.webkit.org/attachment.cgi?id=191459&action=review >>>> Source/WebCore/fileapi/File.cpp:162 >>>> + return modificationTime * 1000.0; >>> >>> nit: Use WTF::msPerSecond instead of 1000.0. >> >> It's for micro-secs per milliseconds, so I defined another constant. > > Really? I think time_t represents seconds in many platforms, and our Date binding code assume a double value as milliseconds. No... sorry I wrote something totally wrong. Changed to use WTF::msPerSecond.
Comment on attachment 191696 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191696&action=review > Source/WebCore/fileapi/File.cpp:157 > + return m_snapshotModificationTime * WTF::msPerSecond; nit: We usually omit "WTF::" prefix. > Source/WebCore/fileapi/File.cpp:162 > + return modificationTime * WTF::msPerSecond; Ditto. > Source/WebCore/fileapi/File.cpp:164 > + return currentTime() * WTF::msPerSecond; Ditto.
Created attachment 191732 [details] Patch for submit
Comment on attachment 191696 [details] Patch Attachment 191696 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17013234 New failing tests: inspector/debugger/debugger-expand-scope.html
Committed r145035: <http://trac.webkit.org/changeset/145035>