Last Comment Bug 99224 - document.lastModified should be localized
: document.lastModified should be localized
Status: RESOLVED FIXED
: fixed1.8, intl
Product: Core
Classification: Components
Component: Internationalization (show other bugs)
: Trunk
: All All
: P3 normal (vote)
: mozilla1.8beta5
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan)
: Yuying Long
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2001-09-10 17:54 PDT by Koike Kazuhiko
Modified: 2005-09-30 07:46 PDT (History)
15 users (show)
mtschrep: blocking1.8b5+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase (191 bytes, text/html)
2001-09-10 17:56 PDT, Koike Kazuhiko
no flags Details
Patch rv1.0 (1.23 KB, patch)
2005-07-26 11:02 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
jst: review+
jst: superreview+
Details | Diff | Review
Patch rv2.0 (3.49 KB, patch)
2005-07-26 14:09 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
jst: review+
jshin1987: review+
brendan: superreview+
mtschrep: approval1.8b5+
Details | Diff | Review
testcase (716 bytes, text/html)
2005-07-26 14:12 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details
Screenshot of the testcase (13.64 KB, image/png)
2005-07-26 14:15 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details
Patch for check-in (3.50 KB, patch)
2005-09-30 07:46 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review

Description Koike Kazuhiko 2001-09-10 17:54:27 PDT
document.lastModified returns GMT, but it should return local time.

IE and N4 return local time.
Comment 1 Koike Kazuhiko 2001-09-10 17:56:19 PDT
Created attachment 48971 [details]
Testcase
Comment 2 Johnny Stenback (:jst, jst@mozilla.com) 2001-09-10 18:07:38 PDT
This is true for http documents, for file documents we get it right.

Targetting for mozilla1.0.
Comment 3 Asa Dotzler [:asa] 2001-12-03 11:06:57 PST
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Comment 4 Prashant Desale 2002-03-15 17:05:16 PST
Severity = LOW [No Crash, Functional failure, No Cosmetic failure]
Visibility = HIGH [Lot of real world website usage, Gets one point of 
compatibility with other browsers since it works on other browsers. gets one 
more point on compliance with adopted techonology, that is JS] 

Priority = Visibility * Severity

Priority = p3

adding word "qawanted" because I'm setting this priority on available data & if 
someone feels otherwise then please investigate this more & feel free to change 
this priority.
Comment 5 Håkan Waara 2002-05-04 12:26:19 PDT
Comment on attachment 48971 [details]
Testcase

This testcase is useless since it doesn't actually show a document's real
lastModified date; only the hardcoded 'unknown' one.
Comment 6 Daniel Wang 2002-10-25 16:39:04 PDT
if this bug is ASSIGNED, should bug 107445 be WONTFIX ?
Comment 7 Daniel Wang 2002-10-25 16:45:08 PDT
missed moz 1.01 deadline
move target milestone -> future
Comment 8 Johnny Stenback (:jst, jst@mozilla.com) 2003-03-23 13:44:45 PST
Mass-reassigning bugs to dom_bugs@netscape.com
Comment 9 Qiang 2004-06-29 09:16:17 PDT
no one is working on this ??  

firefox uses server timezone while shows document.lastModified.

it should follow the client timezone regardless of the server side. IE works 
fine and this bug should be fixed.

visit http://www.easyya.com and see the bottom of the page. It shows GMT 
always.
Comment 10 timeless 2004-11-03 18:09:30 PST
shijialeee@yahoo.com: thank you for volunteering, can you tell us when you'll
have this implemented?
Comment 11 Qiang 2004-11-16 18:34:57 PST
I have no idea why i was assigned the bug. sorry, guys. 
now it's back to the public.
Comment 12 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-07-26 11:02:35 PDT
Created attachment 190579 [details] [diff] [review]
Patch rv1.0

This patch has a problem. But the problem already exists current Trunk.

If document.lastModified value is localized (currently, that is local file and
HTTP header's last-modified header doesn't exist only), the value cannot parsed
by Javascript's Date object.

E.g., if the patch is checked-in, we can see "Not specified" in Modified field
on PageInfo dialog's General tab.
Comment 13 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-07-26 11:15:33 PDT
Currently:
When                        | Localized?
----------------------------+-------------
Local file                  | YES
Existing last-modified      | NO
Not existing last-modified  | YES

After the patch is checked-in:
When                           | Localized?
-------------------------------+-------------
Local file                     | YES
Existing last-modified         | YES
Existing invalid last-modified | NO
Not existing last-modified     | YES
Comment 14 Johnny Stenback (:jst, jst@mozilla.com) 2005-07-26 11:40:53 PDT
Comment on attachment 190579 [details] [diff] [review]
Patch rv1.0

r+sr=jst

Do we need a followup bug filed to fix the page info dialog, or is that already
in bugzilla?
Comment 15 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-07-26 11:49:08 PDT
(In reply to comment #14)
> (From update of attachment 190579 [details] [diff] [review] [edit])
> r+sr=jst
> 
> Do we need a followup bug filed to fix the page info dialog, or is that already
> in bugzilla?
> 

I cannot fine the bugs.
I will file the bugs after check-in (after 1.9 opened).
Comment 16 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-07-26 12:07:27 PDT
Maybe we need to change following code.

http://lxr.mozilla.org/mozilla/source/browser/base/content/pageInfo.js#390
/browser/base/content/pageInfo.js, line 390 -- var modifiedText =
formatDate(theDocument.lastModified, gStrings.notSet);

http://lxr.mozilla.org/mozilla/source/editor/ui/dialogs/content/EdPageProps.js#81
/editor/ui/dialogs/content/EdPageProps.js, line 81 -- lastmod =
editor.document.lastModified; // get string of last modified date

http://lxr.mozilla.org/mozilla/source/js/rhino/docs/faq.html#19
http://lxr.mozilla.org/mozilla/source/js/rhino/docs/scopes.html#23
http://lxr.mozilla.org/mozilla/source/js/rhino/docs/scriptjava.html#19
http://lxr.mozilla.org/mozilla/source/js/rhino/docs/serialization.html#23
/js/rhino/docs/faq.html, line 19 -- var d = new Date(document.lastModified);
/js/rhino/docs/scopes.html, line 23 -- var d = new Date(document.lastModified);
/js/rhino/docs/scriptjava.html, line 19 -- var d = new Date(document.lastModified);
/js/rhino/docs/serialization.html, line 23 -- var d = new
Date(document.lastModified);

http://lxr.mozilla.org/mozilla/source/xpfe/browser/resources/content/pageInfo.js#388
/xpfe/browser/resources/content/pageInfo.js, line 388 -- var modifiedText =
formatDate(theDocument.lastModified, gStrings.notSet);
Comment 17 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-07-26 14:09:31 PDT
Created attachment 190611 [details] [diff] [review]
Patch rv2.0

I propose this approach.
This patch's result is same as IE6 and the result can convert Date object in JS
code.

lastModified always returns "MM/DD/YYYY hh:mm:ss" format. It can parsed by
current JS implementation. If page author needs previous patch's result, they
can write as following.

var date = new Date(document.lastModified);
document.write(date.toLocaleString());

If you grant for this patch, we can fix this bug in 1.8.
Comment 18 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-07-26 14:12:20 PDT
Created attachment 190612 [details]
testcase

In this case, if we use Patch rv2.0, we get more compatibility for IE.
Comment 19 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-07-26 14:15:48 PDT
Created attachment 190614 [details]
Screenshot of the testcase
Comment 20 timeless 2005-07-26 20:04:07 PDT
please say there's going to be an annoucement about this api change somewhere
fairly visible.
Comment 21 Johnny Stenback (:jst, jst@mozilla.com) 2005-08-16 15:21:22 PDT
Comment on attachment 190611 [details] [diff] [review]
Patch rv2.0

r=jst, but I'd like brendan to have a say on this one too.
Comment 22 Brendan Eich [:brendan] 2005-08-19 13:08:20 PDT
So if we're really just emulating IE and unbreaking sites in major locales, then
I'm all for this.  Do we feel the need to do more testing in all locales? 
Cc'ing some folks in case they have thoughts.  I'll sr shortly.

/be
Comment 23 Bob Clary [:bc:] 2005-08-19 13:37:56 PDT
The change seemed very reasonable to me. Did timeless have any concrete objections?
Comment 24 Jungshik Shin 2005-08-21 10:57:03 PDT
(In reply to comment #23)
> The change seemed very reasonable to me. Did timeless have any concrete
objections?

See bug 224703 comment #24. 

Anyway, I don't think MSIE 'hard-coded' 'MM/DD/YYYY HH:MM:SS' (have you tried MS
IE under a different locale (say, French)?) . If you want to hard-code anything,
it had better be ISO 8601-compliant (YYYY-MM-DD ......), but what really need to
be done is to format it according to the locale (corresponding to the language
of a doc.)  of a document (if it's not trivial which is indeed the case, the
current locale). (see bug 224703). 

BTW, it's 'ironic' to see IE use 'UTC' now given that we kinda refrained from
using it partly because of IE 'compatibility' (bug 224744 comment #3, bug 107445)


Comment 25 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-08-29 12:42:57 PDT
Jungshik:

Can you attach better patch? I think that if you cannot create better patch
soon, we should use my patch. Because my patch makes better behavior than
current behavior.

> I don't think MSIE 'hard-coded' 'MM/DD/YYYY HH:MM:SS'

I don't think so. See
http://msdn.microsoft.com/workshop/author/dhtml/reference/properties/lastmodified.asp
Comment 26 Brendan Eich [:brendan] 2005-09-16 16:31:20 PDT
Comment on attachment 190611 [details] [diff] [review]
Patch rv2.0

Unless jshin speaks quickly and has a better patch, I think this should go in
for 1.8b4/Firefox1.5.

/be
Comment 27 Brendan Eich [:brendan] 2005-09-16 16:34:41 PDT
Comment on attachment 190611 [details] [diff] [review]
Patch rv2.0

Requesting explicit second-review+ from jshin.

/be
Comment 28 Johnny Stenback (:jst, jst@mozilla.com) 2005-09-16 17:42:53 PDT
Agreed.
Comment 29 Mike Schroepfer 2005-09-19 14:55:25 PDT
Comment on attachment 190611 [details] [diff] [review]
Patch rv2.0

Approved for 1.8b5 per bug meeting
Comment 30 Scott MacGregor 2005-09-27 15:20:15 PDT
Please get this checked in on the branch and trunk ASAP or this bug will be
removed from the blocking list. Thanks!
Comment 31 Scott MacGregor 2005-09-28 14:26:04 PDT
just another reminder to get this landed ASAP or it may get taken off the
blocker list.
Comment 32 Scott MacGregor 2005-09-29 15:04:51 PDT
Is there anything holding this up from getting checked in? 
Comment 33 Asa Dotzler [:asa] 2005-09-29 18:40:53 PDT
Time is running out. If there's an advocate for this fix with cvs access, now's
the time to step up.
Comment 34 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-29 21:22:16 PDT
Brendan and jst:

Do we need jshin's review? We don't have response from him in a half month.
Comment 35 Jo Hermans 2005-09-30 02:09:38 PDT
This code-pattern with the formated string in the buffer, while very interesting
and self-documenting, isn't thread-safe. Variable formatedTime doesn't point to
the stack, but to anonyous global memory.

+    char formatedTime[] = "MM/DD/YYYY hh:mm:ss";
+    if (sprintf(formatedTime, "%02d/%02d/%04d %02d:%02d:%02d",
+                prtime.tm_month + 1, prtime.tm_mday, prtime.tm_year,
+                prtime.tm_hour     ,  prtime.tm_min,  prtime.tm_sec)) {
+      CopyASCIItoUTF16(nsDependentCString(formatedTime), mLastModified);
+    }

Maybe this is a bit better :

+    // "MM/DD/YYYY hh:mm:ss";
+    char formatedTime[30];
+    if (sprintf(formatedTime, "%02d/%02d/%04d %02d:%02d:%02d",
+                prtime.tm_month + 1, prtime.tm_mday, prtime.tm_year,
+                prtime.tm_hour     ,  prtime.tm_min,  prtime.tm_sec)) {
+      CopyASCIItoUTF16(nsDependentCString(formatedTime), mLastModified);
+    }
Comment 36 Jungshik Shin 2005-09-30 03:48:31 PDT
Comment on attachment 190611 [details] [diff] [review]
Patch rv2.0

Sorry for the delay. I was mistaken that this already had been checked in /
Comment 37 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-30 07:45:55 PDT
checked-in to trunk and 1.8branch.
Thank you everyone!
Comment 38 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-30 07:46:30 PDT
Created attachment 197996 [details] [diff] [review]
Patch for check-in

Note You need to log in before you can comment on or make changes to this bug.