Closed Bug 345643 Opened 14 years ago Closed 14 years ago

Short date display in dayview and weekview are not localizable

Categories

(Calendar :: Calendar Views, defect, P1)

Sunbird 0.3
defect

Tracking

(Not tracked)

RESOLVED FIXED
Sunbird 0.5

People

(Reporter: cedric.corazza, Assigned: mattwillis)

References

()

Details

(Keywords: l12y)

Attachments

(1 file, 7 obsolete files)

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 ?)
CC'ing Axel as it is a l10n matter.
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.
Attached patch use date formatter (obsolete) — Splinter Review
Have we already tried this and dismissed it for some reason?
Assignee: nobody → jminta
Status: NEW → ASSIGNED
Attachment #231025 - Flags: first-review?(mvl)
(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]
Summary: Short date display in dayview and weekview are not editable → Short date display in dayview and weekview are not localizable
*** Bug 347145 has been marked as a duplicate of this bug. ***
*** Bug 347974 has been marked as a duplicate of this bug. ***
Target Milestone: --- → Sunbird 0.5
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.
Assignee: jminta → lilmatt
Attachment #231025 - Attachment is obsolete: true
Attachment #242205 - Flags: second-review?(jminta)
Attachment #242205 - Flags: first-review?(ssitter)
Attachment #231025 - Flags: first-review?(mvl)
Now only does the probing once and stores the order. Yay!
Attachment #242205 - Attachment is obsolete: true
Attachment #242244 - Flags: second-review?(jminta)
Attachment #242244 - Flags: first-review?(ssitter)
Attachment #242205 - Flags: second-review?(jminta)
Attachment #242205 - Flags: first-review?(ssitter)
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.
Attachment #242244 - Attachment is obsolete: true
Attachment #242251 - Flags: ui-review?(dmose)
Attachment #242251 - Flags: second-review?(jminta)
Attachment #242251 - Flags: first-review?(ssitter)
Attachment #242244 - Flags: second-review?(jminta)
Attachment #242244 - Flags: first-review?(ssitter)
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Whiteboard: [cal-ui-review needed]
Version: Sunbird 0.3a2 → Sunbird 0.3
Comment on attachment 242251 [details] [diff] [review]
Figures out the order, but uses text month

Good idea; ui-review+.
Attachment #242251 - Flags: ui-review?(dmose) → ui-review+
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?
(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.


Attached patch fixes ssitter's timezone issue (obsolete) — Splinter Review
Removes jsDate usage to avoid the timezone issue ssitter found.

Carrying forward dmose's ui-review+
Attachment #242251 - Attachment is obsolete: true
Attachment #242328 - Flags: ui-review+
Attachment #242328 - Flags: second-review?(jminta)
Attachment #242328 - Flags: first-review?(ssitter)
Attachment #242251 - Flags: second-review?(jminta)
Attachment #242251 - Flags: first-review?(ssitter)
Whiteboard: [cal-ui-review needed]
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
Attachment #242328 - Flags: first-review?(ssitter) → first-review+
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?
(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.
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.
Attachment #242328 - Attachment is obsolete: true
Attachment #242515 - Flags: ui-review+
Attachment #242515 - Flags: second-review?(jminta)
Attachment #242515 - Flags: first-review+
Attachment #242328 - Flags: second-review?(jminta)
This uses mvl's suggested comparisons to be correct with a wider array of date formats (ie: MMM)
Attachment #242515 - Attachment is obsolete: true
Attachment #242518 - Flags: ui-review+
Attachment #242518 - Flags: second-review?(jminta)
Attachment #242518 - Flags: first-review+
Attachment #242515 - Flags: second-review?(jminta)
Attached patch next trySplinter Review
Fixes undef probeString, and moves longProbeString assignment to before we start changing the date.

Also fixes for loop logic.
Attachment #242518 - Attachment is obsolete: true
Attachment #242519 - Flags: ui-review+
Attachment #242519 - Flags: second-review?(jminta)
Attachment #242519 - Flags: first-review+
Attachment #242518 - Flags: second-review?(jminta)
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.
Attachment #242519 - Flags: second-review?(jminta) → second-review+
Patch checked in on MOZILLA_1_8_BRANCH and trunk.

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Duplicate of this bug: 369858
Duplicate of this bug: 370037
Duplicate of this bug: 373039
Duplicate of this bug: 377129
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.
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.
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.
You need to log in before you can comment on or make changes to this bug.