Closed Bug 377307 Opened 13 years ago Closed 9 years ago
ILocal File shows the wrong last Modified Time
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:18.104.22.168) Gecko/20070309 Firefox/22.214.171.124 XPCOMViewer/0.9.5 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:126.96.36.199) Gecko/20070309 Firefox/188.8.131.52 XPCOMViewer/0.9.5 For certain files the lastModifedTime returns the wrong time. I download a file using NSIWebBrowserPersist and update the lastModifedTime so that I can sync the time with the time of the file on the server. This results with a file with the following timings: Created: Today, April 11, 2007, 5:52:52 PM Modified: Saturday, March 03, 2007, 11:30:38 AM Accessed: Today, April 11, 2007, 6:31:57 PM Unfortunately I can't set/read the created time via nsiLocalFile but all I really need is the last modified time so I ignore the creation time. When I look at the file through the OS. The epoch time is: 1172950238000 however when I use nsiLocalFile.lastModifiedTime it shows the following time: 1172946638000 It's off by exactly one hour (3600 seconds). Somehow an extra hour is either being pushed/removed. Daylight savings code? Reproducible: Always Steps to Reproduce: 1. Create a file with the following timings specified above (c:\temp\foo.txt) Created: Today, April 11, 2007, 5:52:52 PM Modified: Saturday, March 03, 2007, 11:30:38 AM Accessed: Today, April 11, 2007, 6:31:57 PM 2. Create a nsiLocalFile and set it to c:\temp\foo.txt 3. Look at the lastModifiedTime of the nsiLocalFile Actual Results: 1172946638000 Expected Results: 1172950238000
Component: File Handling → XPCOM
Product: Firefox → Core
QA Contact: file.handling → xpcom
This was confirmed on tinderbox today when Bug 178506 landed and failed in exactly the same way on Windows only.
It's worth noting that that test uses the exact same method as comment #0, so nsIWebBrowserPersist may be involved somehow too.
Summary: nsiLocalFile shows the wrong lastModifiedTime → nsILocalFile shows the wrong lastModifiedTime
Version: unspecified → Trunk
This problem is reproducable only when the time zone is using Daylight savings. nsLocalFileWin converts localtime to UTC using SystemTimeToFileTime & LocalFileTimeToFileTime combo. http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsLocalFileWin.cpp#1998 However, SystemTimeToFileTime is not usable to convert localtime because it cannot determine whether the given date is local or not. So it cannot handle Daylight savings correctly. Why nsLocalFile::SetModDate converts UTC to localtime (using PR_LocalTimeParameters), then convert back to UTC again? (using LocalFileTimeToFileTime)
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #467561 - Flags: review?(jmathies)
Comment on attachment 467561 [details] [diff] [review] Use UTC consistently I also consider backporting this to 1.9.2 branch.
Attachment #467561 - Flags: approval2.0?
I think this patch introduces an unexpected behavior change, in that localfile.lastModifiedTime no longer matches the time returned by new Date(). I don't think that this is an acceptable change in general, at least not without some serious changes in the docs and perhaps a change in the method name. It certainly is not something I can approve this close to the FF4 release.
(In reply to comment #6) > I think this patch introduces an unexpected behavior change, in that This patch doesn't change the behavior. This patch just fixes the inconsistency between SetLastModifiedTime and GetLastModifiedTime. > localfile.lastModifiedTime no longer matches the time returned by new Date(). What the word "match" means? This patch doesn't change GetLastModifiedTime at all. This patch just use fix the bug of SetLastModifiedTime on Windows. > I don't think that this is an acceptable change in general, at least not > without some serious changes in the docs and perhaps a change in the method > name. This is a Windows only behavior. Do you say that Firefox for Mac and Linux have the bug? Or is there a document which states about the difference between platforms?
This patch doesn't break the following test (see xpcom/tests/unit/test_localfile.js). I don't really understand what "localfile.lastModifiedTime no longer matches the time returned by new Date()" means. > var now = Date.now(); > file.create(Ci.nsIFile.NORMAL_FILE_TYPE, 0644); > do_check_true(file.exists()); > > // Modification time may be out by up to 2 seconds on FAT filesystems. Test > // with a bit of leeway, close enough probably means it is correct. > var diff = Math.abs(file.lastModifiedTime - now); > do_check_true(diff < MAX_TIME_DIFFERENCE);
(In reply to comment #5) > I also consider backporting this to 1.9.2 branch. If this gets backported to 1.9.2 please also backport to 1.9.1.
Comment on attachment 467561 [details] [diff] [review] Use UTC consistently renominating per comment 7
Attachment #467561 - Flags: approval2.0- → approval2.0?
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
Funny. In bug 226793, I had created almost exactly the same patch in 2003, but the patch got stuck in too long review queues - 1 year. By the time it was reviewed, the patch had rotted and I had moved on to other projects.
You need to log in before you can comment on or make changes to this bug.