Closed Bug 38014 Opened 20 years ago Closed 18 years ago

FTP dates should be shown as localized dates

Categories

(Core :: Networking: FTP, defect, P3)

defect

Tracking

()

VERIFIED FIXED
mozilla0.9.6

People

(Reporter: bugzilla, Assigned: bbaetz)

References

Details

(Keywords: testcase)

Attachments

(4 files)

When you go to fx:
ftp://ftp.mozilla.org/pub/mozilla/nightly/latest/
the dates on the files should be shown as set up in my windows control panel.

So because I'm running a english version 2k localized to Danish, the dates 
should be shown like this:
02-05-2000
instead of:
05-02-2000

Build 2000050215
->jud
Assignee: gagan → valeski
Frank - I'm using the "GetApplicationLocale()" method to generate the date in
FTP for the http index output (see
http://lxr.mozilla.org/seamonkey/source/netwerk/streamconv/converters/nsFTPDirListingConv.cpp#393).
Should I be using something else?

Chris - could this formatting be getting lost in the dir viewer?
http://lxr.mozilla.org/seamonkey/source/xpfe/components/directory/nsDirectoryViewer.cpp#802
Target Milestone: --- → M30
Ftp dates are not working all the time anyway as there are problems with some 
international Win versions to display these dates at all.
(bug 30994 FTP listing does not show "last modified")
robert-- welcome to necko!
Assignee: valeski → rjc
Target Milestone: M30 → Future
Hi dougt, welcome to necko :)
Assignee: rjc → dougt
Blocks: 62354
Has something been done? I'm now getting "04-01-2001" as the date, which seems 
right.
Component: Networking → Networking: FTP
Shoudln't dates be shown according to the *Mozilla* locale (i.e., current 
language pack), not the system locale?
*Mozilla* locale makes no sense. I always install english version of all 
software. But I like that my English Excel is using the settings from my Region 
settings in my Control Panel when showing numbers, etc...
according the reporters comments on 2001-01-11, this has been fixed.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
can someone with a us machine please verify that the dates gets shown correct.
If I go to:
ftp://ftp.mozilla.org/
the dates shown on my locale=danish gets shown like this:
"README 0 11/23/99 12:00:00 am symbolic-link"
I see US datetimes in the FTP listing, and I have US Windows set to UK region.
not fixed.
If I go to ftp://ftp.mozilla.org
the dates are shown as "11/23/99"
I'd expect "23/11/99"

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
qa to me.
QA Contact: tever → benc
reassigning to bbaetz@cs.mcgill.ca.
Assignee: dougt → bbaetz
Status: REOPENED → NEW
Done as part of 78148
Status: NEW → ASSIGNED
Depends on: 78148
Target Milestone: Future → mozilla0.9.6
Mass change: Bug 78148 has been checked in, and so it and these dependancies
should have been fixed. If you disagree, please reopen.

Note that the xul view is still enabled for file because of i18n problems. The
bug to fix this is bug 102812.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
not really fixed.
Dates are now shown, using an english Win2k box localized to Danish, like this:
Mozilla     : 10/02/01 11:51:00
Netscape 4.x: Tue Oct 02 11:51:00 2001
IE          : 02-10-2001 11:51

The dateformat "10/02/01" is not correct. It should be like IE style 
"02-10-2001"
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This definately works for me on linux, when I set the locale to nl_NL. (I don't
know what the ISO country code for denmark is)

I just ask for the application locale from the OS. How did you localise moz?
Using the windows regional settings stuff, or something else?
My english Win2k is localized using the control panels "windows regional 
settings"...
This should work, then. You are using today's nightly, right?

Do they appear correctly in the xul view? (prefs->debug->networking panel has
the option)

I'll ask some i18n people.
So do any dates in mozilla appear localised properly? If so, let me know which
ones, and I'll go look at that code. If not, this is not an ftp bug.
OS: Windows 2000 → All
Hardware: PC → All
+testcase - QA will check dates as part of functional tests now.
Keywords: testcase
smfr said that it was wrong for him on mac, too. He was going to try changing
the call to mDateTie to pass in nsnull as a locale, which is what everyone else
does, even though thats not what the docs say.

smfr?
Status: REOPENED → ASSIGNED
Yes, you should just pass nsnull as the first parameter to FormatPRTime().
That patch fixes a number of issues:
1. It passes nsnull as the first param of FormatPRTime, thus using system
   date/time settings.
2. It fixes the bad HTML (<pre> around everything replaced with <tt>, quoting
   of attributes, consistent attribute case).
3. It splits date and time into separate table columns, so that they can be
   nicely right-aligned.
4. It shows file sizes as multiples of 1K, which is much more readable.
5. It fixes the alignment of the item label text w.r.t the image.
6. It add some cellpadding for a bit of extra space around items.
7. It collapses some pairs of pushBuffer.Append() calls into single calls for
   efficiency.

FTP listings now look pretty good. The few things I'd like to see fixed now:
 i) symlinks should have a different icon (or icons: symlink to file, symlink 
    to dir).
ii) more icon types?
shouldn't " KB" be in some external resource file?

> shouldn't " KB" be in some external resource file?
I'd be suprised if this is something that would be localized. "KB" is a pretty 
standard measure of file size in all languages, no?
Of course, there are other strings here that need localization, like "Index of".
This file is localised as of last Friday, so you'll need to merge. :)

Not all the ftp backends identify symlinks in directory listings (and not all
servers tell us about them)

Re icon types, I have a bug somewhere in my "rewrite the xul viewer" list to use
the icon: url stuff. I need examples of that, though. Mozclassic did extention
matching to display what it though the type was (from the mime handlers). I
don't know if doing that is worth it.

-    pushBuffer.Append(NS_LITERAL_STRING("\"> <img border=\"0\"
align=\"absbottom\" src=\""));
+    pushBuffer.Append(NS_LITERAL_STRING("\"> <img border=\"0\" align=\"middle\"
hspace=\"2\" src=\""));

ns4 does absbottom - I just copied.

If a file's size < 1024, we should display the size in bytes. Possibly try once
more, and do Mb, too? I don't think that it does need translating, but we should
check with someone.

You also don't need the extra protected: in the .h.

Apart from that, looks great! Thanks a lot.
Simon, do you want to take this, or do you want me to merge it in? Note that if
we're not going to use mlocale, then you can remove the member variable and the
stuff from Init.
bbaetz: it's all yours. I'll leave it to you to remove the mLocale stuff. Let me 
know if you need sr.
Attached patch updated patchSplinter Review
OK, I've updated smfr's patch to apply to the trunk. After talking to him on
IRC, I agree with how he's done the formatting of the output.
Comment on attachment 55688 [details] [diff] [review]
updated patch

I like it! sr=sfraser
Attachment #55688 - Flags: superreview+
Comment on attachment 55688 [details] [diff] [review]
updated patch

>Index: nsIndexedToHTML.cpp

>-    nsCOMPtr<nsILocaleService> localeServ = do_GetService(NS_LOCALESERVICE_CONTRACTID);
>-    localeServ->GetApplicationLocale(getter_AddRefs(mLocale));

can mLocale be removed then?


>     buffer.Append(NS_LITERAL_STRING("</H1>\n"));
>-    buffer.Append(NS_LITERAL_STRING("<hr><table border=0>\n"));
>+    buffer.Append(NS_LITERAL_STRING("<hr><table border=\"0\" cellpadding=\"2\">\n"));

how about combining these two literal appends?


>+    pushBuffer.Append(NS_LITERAL_STRING("</tt></a></td>\n"));
>+    pushBuffer.Append(NS_LITERAL_STRING(" <td align=\"right\"><tt>"));

how about combining these two literal appends too?


>-        pushBuffer.AppendInt(size);
>+        nsAutoString  sizeString;
>+        FormatSizeString(size, sizeString);
>+        pushBuffer.Append(sizeString);
>+        pushBuffer.Append(NS_LITERAL_STRING("&nbsp;"));   // for a little extra space
>     } else {
>         pushBuffer.Append(NS_LITERAL_STRING("&nbsp;"));
>     }
>-
>-    pushBuffer.Append(NS_LITERAL_STRING("</td>\n"));
> 
>-    pushBuffer.Append(NS_LITERAL_STRING(" <td>"));
>-    
>+    pushBuffer.Append(NS_LITERAL_STRING("</tt></td>\n <td align=\"right\"><tt>"));

how about combining the Append for the "&nbsp;" with this one?



>     if (t == -1) {
>         pushBuffer.Append(NS_LITERAL_STRING("&nbsp;"));
>+        pushBuffer.Append(NS_LITERAL_STRING("</tt></td>\n <td><tt>"));
>+        pushBuffer.Append(NS_LITERAL_STRING("&nbsp;"));

how about combining these?



>     } else {
>-        nsAutoString formatted;
>-        mDateTime->FormatPRTime(mLocale,
>+        nsAutoString formattedDate;
>+        mDateTime->FormatPRTime(nsnull,
>                                 kDateFormatShort,
>+                                kTimeFormatNone,
>+                                t,
>+                                formattedDate);
>+        pushBuffer.Append(formattedDate);
>+        pushBuffer.Append(NS_LITERAL_STRING("</tt></td>\n <td align=\"right\"><tt>"));
>+        nsAutoString formattedTime;

why not reuse formattedDate?  and call it something more generic perhaps.


>+        mDateTime->FormatPRTime(nsnull,
>+                                kDateFormatNone,
>                                 kTimeFormatSeconds,
>                                 t,
>-                                formatted);
>-        pushBuffer.Append(formatted);
>+                                formattedTime);
>+        pushBuffer.Append(formattedTime);
Attachment #55688 - Flags: needs-work+
> can mLocale be removed then?
It is, I just forgot to diff the header file. Oops.

> how about combining these two literal appends too?

OK, will do; new patch tomorrow.
This also fixes bug 99169. New patch coming
Blocks: 99169
Keywords: patch, review
Attached patch patchSplinter Review
Comment on attachment 56208 [details] [diff] [review]
patch

lowercase the tags (H1, HREF) and sr=sfraser
Attachment #56208 - Flags: superreview+
Comment on attachment 56208 [details] [diff] [review]
patch

r=darin
Attachment #56208 - Flags: review+
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago18 years ago
Resolution: --- → FIXED
v 20020222
Status: RESOLVED → VERIFIED
+testcase - more work for FTP test suite.
(In reply to comment #28)
> > shouldn't " KB" be in some external resource file?
> I'd be suprised if this is something that would be localized. "KB" is a pretty 
> standard measure of file size in all languages, no?
> Of course, there are other strings here that need localization, like "Index of".

It actually does need to be localisable. Not every language uses the Latin
alphabet, therefore, the string will look different in Cyrillic, let's say. 

I think we should add a common interface for "KB", so that it's localised only
in one place, and used all over mozilla. 
You need to log in before you can comment on or make changes to this bug.