345643 - Short date display in dayview and weekview are not localizable

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect, P1)

Description

[u162289]

The shortdate displayed above the day noun in the main panel for the dayview and the weekview is formatted MM/DD but in some locales, it should be displayed DD/MM .
This should be localizable (maybe via a preference or relying on the OS ?)

Comment 1

[u162289]

CC'ing Axel as it is a l10n matter.

Comment 2

[Stefan Sitter [:ssitter]]

We use calIDateTimeFormatter that uses nsIScriptableDateFormat to display date and times in local format. In the day/week view this is currently hard coded. In Bug 322059 I tested the local short format but that does not give sufficient results because it always includes the year. So we probably need a version of calIDateTimeFormatter/nsIScriptableDateFormat that excludes the year.

Comment 3

[Joey Minta]

Attachment: use date formatter

Have we already tried this and dismissed it for some reason?

Comment 4

[Stefan Sitter [:ssitter]]

(In reply to comment #3)
> Have we already tried this and dismissed it for some reason?

Unfortunately this does not fix the problem. After this patch the date format will be still hardcoded but look like 'Jul 28'. 
[http://lxr.mozilla.org/mozilla/source/calendar/base/src/calDateTimeFormatter.js#133]

Comment 5

[Martin Schröder [:mschroeder]]

*** Bug 347145 has been marked as a duplicate of this bug. ***

Comment 6

[Stefan Sitter [:ssitter]]

*** Bug 347974 has been marked as a duplicate of this bug. ***

Comment 7

[Matthew (lilmatt) Willis]

Attachment: figures out the MM/DD vs. DD/MM order the hard way

This uses a known sample date and formats it to determine the correct MM/DD vs. DD/MM order, and then formats the passed date appropriately, without the year.

It could probably be optimized using a mYearlessShortDateFormat var or something so we only do the sample formatting once per launch.

Comment 8

[Matthew (lilmatt) Willis]

Attachment: with the aforementioned optimization now included

Now only does the probing once and stores the order. Yay!

Comment 9

[Matthew (lilmatt) Willis]

Attachment: Figures out the order, but uses text month

After talking with jminta about this, he suggested using the "Oct 3" style of date over the "10/3" style, as it eliminates the "is 10/3 October 3rd, or March 10th?" question entirely. ui-review? for this change.

This patch uses the MM/DD information to determine which order to put the "Oct" and the "3" in.

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 242251 [details] [diff] [review]
Figures out the order, but uses text month

Good idea; ui-review+.

Comment 11

[Stefan Sitter [:ssitter]]

Comment on attachment 242251 [details] [diff] [review]
Figures out the order, but uses text month

>-    probeDate.jsDate = new Date(2002,3,4);
>+    probeDate.jsDate = new Date(2002,3,5);
>     try { 
>         var probeString = this.formatDateShort(probeDate);
>+        // Note that we're checking for "4" instead of the "3" we set because
>+        // a jsDate's month is zero-based.
>+        this.mMonthFirst =
>+                (probeString.indexOf("4") < probeString.indexOf("5"));
>+

This check doesn't work right. Regardless of my short format being 'DD.MM.YYYY' or 'YYYY.MM.DD' the result is always 'DD MMM'. Dump output shows the problem:

XXXdump probeDate.jsDate=Fri Apr 05 2002 00:00:00 GMT+0200
XXXdump probeDate=2002/04/04 22:00:00 UTC
XXXdump probeString=2002.04.04
XXXdump probeString.indexOf('4')=6
XXXdump probeString.indexOf('5')=-1
XXXdump mMonthFirst=false

There also exist users/systems that use 'MMM' in short date format instead of 'MM'. How to accomplish this?

Comment 12

[Matthew (lilmatt) Willis]

(In reply to comment #11)
> There also exist users/systems that use 'MMM' in short date format instead of
> 'MM'. How to accomplish this?

In IRC, I asked ssitter which systems use "MMM" in their short date, and he pointed out the system in bug 326095.

While his point is true, I'd argue it's in the minority compared to the large number of people commenting that they want "3/10" instead of "10/3".

So after I fix the UTC-related issue ssitter found with my probe, I think we should take it as is and make fixing it to deal with "MMM" part of bug 326095 or bug 257146.

Comment 13

[Matthew (lilmatt) Willis]

Attachment: fixes ssitter's timezone issue

Removes jsDate usage to avoid the timezone issue ssitter found.

Carrying forward dmose's ui-review+

Comment 14

[Stefan Sitter [:ssitter]]

Comment on attachment 242328 [details] [diff] [review]
fixes ssitter's timezone issue

Add back the changes to calendar/base/content/calendar-multiday-view.xml

>+++ calendar/base/src/calDateTimeFormatter.js	15 Oct 2006 14:52:45 -0000
>@@ -45,25 +45,36 @@
>+    // Do we format MM/DD or DD/MM in this locale?
>+    this.mMonthFirst = false;

Please update comment because we don't do MM/DD or DD/MM

r=ssitter with this fixed

Comment 15

[Joey Minta]

Comment on attachment 242328 [details] [diff] [review]
fixes ssitter's timezone issue

+    // Do we format MM/DD or DD/MM in this locale?
+    this.mMonthFirst = false;
+

I'm having trouble wrapping my head around the re-assignment of mMonthFirst and the try/catch block.  Specifically, we seem to have a violation of the general rule "try/catch the smallest amount of code possible"  My question is: Will it ever be the case that the +        this.mMonthFirst =
+                (probeString.indexOf("4") < probeString.indexOf("5"));
lines won't be reached?  If they will always be reached they should be pulled outside the try/catch. More generally, which line in this block is expected to throw?

Comment 16

[Matthew (lilmatt) Willis]

(In reply to comment #15)
> My question is: Will it ever be the case that the
> +        this.mMonthFirst =
> +                (probeString.indexOf("4") < probeString.indexOf("5"));
> lines won't be reached?
If the assignment to probeString throws, then they won't.

> More generally, which line in this block is expected to throw?
I _think_ it's the probeString assignment line but I didn't write the try/catch block originally.

The formatting seems like the only thing that will throw.

Comment 17

[Matthew (lilmatt) Willis]

Attachment: Has ssitter's nit plus missing XBL piece

This is the same as attachment 242328 [details] [diff] [review] except it has the missing XBL piece from attachment 242251 [details] [diff] [review], and ssitter's comment nit addressed.

Comment 18

[Matthew (lilmatt) Willis]

Attachment: Uses mvl's more complex comparison for wider coverage

This uses mvl's suggested comparisons to be correct with a wider array of date formats (ie: MMM)

Comment 19

[Matthew (lilmatt) Willis]

Attachment: next try

Fixes undef probeString, and moves longProbeString assignment to before we start changing the date.

Also fixes for loop logic.

Comment 20

[Joey Minta]

Comment on attachment 242519 [details] [diff] [review]
next try

You also need to update the idl, since you're changing the result returned.

+        // probeStringA to probeStringB and probeStringA to probeStringC. 
+        for (var i=0; i < probeStringA.length ; i++) {
Spaces around operators (=) please, and no space after length.

r2=jminta with those.

Comment 21

[Matthew (lilmatt) Willis]

Patch checked in on MOZILLA_1_8_BRANCH and trunk.

-> FIXED

Comment 26

[Beryl Jack]

PLEASE, PLEASE FIX THIS:  We're NOT all American and the US date format at the top of the calendar is causing grief to all of the users at our university. We would love to be able to set preference to Jul 3, Oct 24 etc. although we would prefer 3 Jul, Oct 24.... anything but 7/3, 10/24 etc.

Comment 27

[Beryl Jack]

PLEASE, PLEASE FIX THIS:  We're NOT all American and the US date format at the top of the calendar is causing grief to all of the users at our university. We would love to be able to set preference to Jul 3, Oct 24 etc. although we would prefer 3 Jul, Oct 24.... anything but 7/3, 10/24 etc.

Comment 28

[Stefan Sitter [:ssitter]]

Beryl Jack, I'd recommend to read the Bug you are commenting on. This would give you the information that this bug is already fixed and will be included in the Sunbird 0.5 / Lightning 0.5 release.