364672 - Minimonth: Chinese weekday names indistinguishable; even after expanding sidebar

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Daniel Yee]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: http://download.mozilla.org/?product=lightning-0.3&os=win

When using Chinese WinXP, but using English version of Thunderbird and Lightning, the week names are incomplete. Only shows first two characters of the three character names. Most likely a display issue as Sunbird .3a2 displayed these correctly.

Reproducible: Always

Steps to Reproduce:
1. Install English version of Lightning into English version of Thunderbird on Chinese WinXP.


Actual Results:  
Lightning uses local language settings, but mis-displays week titles

Expected Results:  
should be three character week titles (星期一，星期二，星期三，星期四，星期五，星期六，星期日)

Theme: CrossOver theme 2.3 (still occurs with default theme)

Comment 1

[Daniel Yee]

large calendar view uses English localization, while this bug only affects the mini calendar view in the mail toolbar.

Comment 2

[gekacheka]

Attachment: image illustrating problem

Comment 3

[gekacheka]

Attachment: image illustrating fix

Minimonth tries to use a 1 or 2 character abbreviation to keep the picker compact.  

Chinese calendars often use just a single character for the weekday: the numbers 1-6 (一、二、三、四、五、六)　or 'sun' (日) (you can search Google images for "Chinese calendar" to see examples).  

The long form is 星期一，星期二，星期三，星期四，星期五，星期六，星期日.

Comment 4

[gekacheka]

Attachment: v1 patch minimonth to trim common prefix from weekday abbrevs for chinese

(patch -l -p 1 file.patch)

To keep compact, Minimonth tries to create 1 or 2 character abbreviations for weekdays.  Shorter than normal abbreviations are possible because it is obvious these are weekday abbreviations from their context (heading of columns in month grid).

Approach: If weekday abbreviations are longer than two characters and share a common prefix, trim the prefix.  (Then on the remainder, trim any characters after the 1 or 2 character unique abbrev, as before.)

This should work for all locales listed in http://lxr.mozilla.org/l10n/search?string=ddd
Note that the minimonth abbreviation comes from the operating system, not these localization files, but since many locales have been localized, this provides some confidence that this is not a widespread issue.  The only other of these locales which has a common prefix at all is ca (Catalan), where each day abbrev starts with 'd', but each day abbrev is only two characters long so it doesn't need to be shortened.

Tested on en-US w2k as follows:
  Settings > Control Panel > Regional Options:  
    Language Settings for the System:  
      checked Simplified Chinese, click ok, (may need CD to install fonts)
      restarted system.
  Settings > Control Panel > Regional Options:
    Your Locale:  Chinese (Singapore)
    This sets date and time formats but not menu strings.
  Now starting Sunbird produces Minimonth where weekday headings are in Chinese.
  Reverse above changes to get back to your previous date formats; 
   (If you customized them in date or time tab, 
    you'll have to re-enter the previous format.  
    I like setting long date format to "ddd, d MMM yyyy" so the dates show
    day-of-week in the Sunbird event-list, and use abbreviated day-of-week 
    and month names so they are not too wide.)

Comment 5

[Matthew (lilmatt) Willis]

Comment on attachment 250050 [details] [diff] [review]
v1 patch minimonth to trim common prefix from weekday abbrevs for chinese

Moving review to ctalbert since he has Windows and Asian character experience.

Comment 6

[cmtalbert]

Comment on attachment 250050 [details] [diff] [review]
v1 patch minimonth to trim common prefix from weekday abbrevs for chinese

>+            if (dayList.some(function(dayAbbr){ return dayAbbr.length > 2; })) {
>+              for (var endPrefix = 0; true; endPrefix++) {
>+                var c = dayList[0][endPrefix];
>+                if (dayList.some(function(dayAbbr) {
>+                                   return dayAbbr[endPrefix] != c; })) {
if all the dayAbbr names are identical, will this if will ever evaluate to true? Because if this statement does not evaluate to true, then we will be stuck in an infinite loop.  This is not a very likely scenario, but we should be aware of that danger nonetheless.  An else block for this if could solve this problem, and I don't think it would over complicate the code.

>+            // 2. trim each day abbreviation to a unique 1 or 2 chars.
>+            for (i = 0; i < dayList.length; i++) {
>+              var foundMatch = 1;
I'm not sure if it matters in JS, but the original code here declared foundMatch outside the first loop.  You chose to declare it inside. Does this have any affect on memory usage in JS?  If not, I'm all for eliminating unneeded lines of code. I just am not sure if there are any side effects from this practice.

I have tested this on Windows XP Chinese and found that this patch solves the problem. So other than my one nit and question above, this patch looks good.

ctalbert r+ (with the nit and question addressed)

Comment 7

[gekacheka]

Attachment: v2 patch minimonth to trim common prefix from weekday abbrevs for chinese (nit addressed)

(patch -l -p 1 file.patch)

Now computes the minimum length day name and limits the loop to that length.
(before it assumed review process would make sure no locale uses the same string for all weekdays, so limit would never be reached).

The var declaration in loop should not make any performance difference in general.  Javascript treats all var declarations as if they are promoted to the top level of the function.  (In this particular case, the loop is so rare and short the performance difference would not be significant anyway.  Readability is more important.  Declaring the var inside the loop indicates intended scope of use: the value is used only within one iteration and is not needed for the next iteration.)

Comment 8

[Simon Paquet [:sipaq]]

Comment on attachment 253095 [details] [diff] [review]
v2 patch minimonth to trim common prefix from weekday abbrevs for chinese (nit addressed)

Moving this review to Mickey, since Joey is pretty busy at the moment and we still need a 2nd review since clint is not a module peer.

Comment 9

[Stefan Sitter [:ssitter]]

Is this patch still required? 
After checkin for Bug 317607 the weekday names are now retrieved from localization language and not OS language. So it's up to the l10n team for Chinese to pick the correct abbreviations.

Comment 10

[Martin Schröder [:mschroeder]]

Stefan, maybe this patch is still needed for the OS format fallback. What do you think?

Comment 11

[Michael Büttner (no reviews TFN)]

Comment on attachment 253095 [details] [diff] [review]
v2 patch minimonth to trim common prefix from weekday abbrevs for chinese (nit addressed)

I agree with Martin here, it's not strictly required but could bump in if we're using the OS names. r=mickey

Comment 12

[Stefan Sitter [:ssitter]]

Doesn't this patch conflict with the patch in Bug 373350?

Comment 13

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Checked in merged patch from bug 373350 on HEAD and MOZILLA_1_8_BRANCH

-> FIXED