275960 - document.lastModified cannot handle non-ascii characters when the page's HTTP header doesn't have Last-Modified

Status: RESOLVED FIXED

(Core :: Internationalization, defect, P1)

Description

[Riven Cui]

USER-AGENT:Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a6)
Gecko/20041224 Firefox/1.0+

The document.lastModified property is expected to return an localized string for
the date. But on my system (Windows XP Pro. Simplified Chinese), the result
return by it is unreadable.

Reproductivity: Always

Step to reproduce:
1.Download the testcase
2.Open it on a non-English windows version.

Comment 1

[Riven Cui]

Attachment: Please run it on non-English Windows

Comment 2

[Riven Cui]

Attachment: Result unreadable in firefox and mozilla

Comment 3

[Riven Cui]

Attachment: Expected result

Comment 4

[Boris Zbarsky [:bzbarsky]]

So... last-modified over HTTP is ascii.  Where is the non-ascii coming from in
this case?  Note that nsDocument::GetLastModified uses CopyASCIItoUCS2, so it's
certainly assuming it has an ASCII string.  Can PR_FormatTime() give us
non-ascii data?

Comment 5

[dynamis (Tomoya ASAI)]

Garbage Characters not always shown.
It seems that we will get garbase characters with only page who don't return
Last-Modified HTTP header.
# It seems that without the header, Firefox will generate garbage.
This wasn't happen with aviary builds.

For example, with firefox start page, we get garbase with
  javascript: alert( document.lastModified );

HTTP Header is like this (captured with httpheaders):
----------------------------------------------------------
http://www.google.co.jp/firefox

GET /firefox HTTP/1.1
Host: www.google.co.jp
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.8b3)
Gecko/20050710 Firefox/1.0+
Accept:
text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5
Accept-Language: ja,en-us;q=0.7,en;q=0.3
Accept-Encoding: gzip,deflate
Accept-Charset: Shift_JIS,utf-8;q=0.7,*;q=0.7
Keep-Alive: 300
Connection: keep-alive
Cookie:
PREF=ID=87cfcb29483aa85f:LD=ja:TM=1116700911:LM=1116722731:S=wEm9xsFaOCSyIAOF

HTTP/1.x 200 OK
Cache-Control: private
Content-Type: text/html
Content-Encoding: gzip
Server: GWS/2.1
Content-Length: 1765
Date: Wed, 20 Jul 2005 03:14:12 GMT
----------------------------------------------------------

Comment 6

[dynamis (Tomoya ASAI)]

Attachment: alert with garbage characters

open the page: http://www.google.co.jp/firefox
exec javascript from location bar: javascript: alert( document.lastModified );
and this dialog will be shown.

Comment 7

[dynamis (Tomoya ASAI)]

Attachment: alert with correct characters

Same with Aviary Firefox (dialog have correct characters).

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 5/2-5/10)]

Attachment: Patch rv1.0

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 5/2-5/10)]

This may be regression of bug 116598.
We should fix before 1.8b4.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 189908 [details] [diff] [review]
Patch rv1.0

This doesn't seem like the right fix, since in general there is no reason this
string would be in the native encoding... For cases when we format it from the
current date that would be the case, but that's not always happening.

Ideally, we would store this string as UTF8, always.

Comment 11

[Jungshik Shin]

(In reply to comment #10)

> Ideally, we would store this string as UTF8, always.

Either that or UTF-16.  A better fix would be to use a method of nsIDateTimeFormat 

(http://lxr.mozilla.org/seamonkey/source/intl/locale/public/nsIDateTimeFormat.h )
in nsDocument::RetrieveRelevantHeaders unless using '%c' is absolutely required,
which I don't think is the case.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 5/2-5/10)]

Attachment: Patch rv2.0

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 5/2-5/10)]

Attachment: Patch rv2.1

Comment 14

[Jungshik Shin]

Comment on attachment 190002 [details] [diff] [review]
Patch rv2.1


>+      CopyASCIItoUCS2(tmp, mLastModified);

nit: CopyASCIItoUTF16 

>
>     modDate = PR_Now();
>   }
> 
>   if (LL_NE(modDate, LL_ZERO)) {
>-    PRExplodedTime prtime;
>-    char buf[100];
>-
>-    PR_ExplodeTime(modDate, PR_LocalTimeParameters, &prtime);

  I think you'd better keep this so that |mLastModified| is represented in the
local time zone.

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 5/2-5/10)]

Attachment: Patch rv3.0

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 5/2-5/10)]

Comment on attachment 190024 [details] [diff] [review]
Patch rv3.0

The risk is low. But this is important for intl.

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 5/2-5/10)]

checked-in.