449646 - Edit Recurrence dialog is broken in some localizations

Status: RESOLVED WONTFIX

(Calendar :: General, defect)

Description

[Stefan Sitter [:ssitter]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; pt-BR; rv:1.8.1.17pre) Gecko/2008080621 Calendar/0.9pre

Mozilla/5.0 (Windows; U; Windows NT 5.1; nb-NO; rv:1.8.1.17pre) Gecko/2008080621 Calendar/0.9pre

The Edit Recurrence dialog is broken if an error happens during changing the order of the dialog items depending on the translation of "monthlyOrder", "yearlyOrder" or "yearlyOrder2".

An error happens if the translation adds additional content to the string, e.g. as currently done by e.g. nb-NO, ro, nn-NO, pt-BR.

http://mxr.mozilla.org/l10n-mozilla1.8/search?string=monthlyOrder
http://mxr.mozilla.org/l10n-mozilla1.8/search?string=yearlyOrder

A translation like "yearlyOrder=%2$S de %1$S" or "yearlyOrder2=%2$S, %1$S %4$S %3$S" breaks the initialization with the error

Error: newEl has no properties
Source File: chrome://calendar/content/sun-calendar-event-dialog-recurrence.js
Line: 612

because it can't find an element with ID "de" or "yearly-weekday,".

The dialog should be able to handle such errors e.g. by falling back to the default item order and finishing the initialization. Or there should be an explicit localization note that forbids adding new content to the string.

Comment 1

[Emerson Prado]

IMHO, there should be a way for adding those details. yearlyOrder2 has a place for the "of" translation, maybe a similar solution can be used for yearlyOrder, and also for the commas, etc...

Comment 2

[Stefan Sitter [:ssitter]]

Attachment: proposed workaround to minimize the failure

This patch doesn't fix the error. But it minimize the failure by moving the changeWidgetsOrder() to the end of onLoad(). Now the dialog is at least correctly initialized when the error happens. It's possible to work with it and change the recurrence rule. Only the affected controls might not be in the desire position/order.

Comment 3

[Simon Paquet [:sipaq]]

Guys,
please take a look at the two links listed in the bug description here in comment 0. Please remove any additional characters in line 149 of sun-calendar-event-dialog.properties (like the "," in the ro localization or the "." in the two Norwegian localizations) from the file.

Thanks!

Comment 4

[RickieES aka Ricardo Palomares]

(In reply to comment #3)
> Guys,
> please take a look at the two links listed in the bug description here in
> comment 0. Please remove any additional characters in line 149 of
> sun-calendar-event-dialog.properties (like the "," in the ro localization or
> the "." in the two Norwegian localizations) from the file.


Reviewed for es-ES, it was already OK.

Comment 5

[Alexandru Szasz]

Where should I commit the changes? I can't find calendar in hg.

Comment 6

[Rimas Kudelis [use needinfo to ping]]

AFAICS it's on CVS TRUNK and MOZILLA_1_8_BRANCH so far.

Comment 7

[Håvard Mork]

Fixed for nb-NO and nn-NO on cvs trunk, 1.8 branch, and in hg.

Comment 8

[Rimas Kudelis [use needinfo to ping]]

Oh right, Calendar l10n files in l10n-central already. Should be fixed there too, when so.

Comment 9

[Alexandru Szasz]

Same as comment 7 for ro

Comment 10

[Simon Paquet [:sipaq]]

(In reply to comment #7)
> Fixed for nb-NO and nn-NO on cvs trunk, 1.8 branch, and in hg.

Håvard, can you please fix these four locations on cvs trunk, 1.8 branch and hg as well?

http://mxr.mozilla.org/l10n-mozilla1.8/source/nb-NO/calendar/chrome/prototypes/sun-calendar-event-dialog.properties#144
http://mxr.mozilla.org/l10n-mozilla1.8/source/nn-NO/calendar/chrome/prototypes/sun-calendar-event-dialog.properties#144
http://mxr.mozilla.org/l10n-mozilla1.8/source/nb-NO/calendar/chrome/prototypes/sun-calendar-event-dialog.properties#140
http://mxr.mozilla.org/l10n-mozilla1.8/source/nn-NO/calendar/chrome/prototypes/sun-calendar-event-dialog.properties#140

There are still additional characters in the sun-calendar-event-dialog.properties file (like "."). Thanks.

Comment 11

[Håvard Mork]

Simon, MXR shows the previous revision of the files. Perhaps MXR isn't updated yet? I'm pretty sure I checked in correctly.

Comment 12

[Simon Paquet [:sipaq]]

(In reply to comment #11)
> Simon, MXR shows the previous revision of the files. Perhaps MXR isn't updated
> yet? I'm pretty sure I checked in correctly.

You're right. Sorry about that. I should have checked with bonsai.

Back to the bug:
Daniel, Stefan, now that all the locales have been fixed do we still consider this bug a release blocker? Do we still need the workaround?

Comment 13

[Hubert Gajewski]

Attachment: Edit Recurrence dialog window

Emerson, do you need something like this?

Comment 14

[Daniel Boelzle [:dbo]]

Comment on attachment 332947 [details] [diff] [review]
proposed workaround to minimize the failure

I don't think we should try to hide translation problems; I event think it's wanted that those show up immediately.

Comment 15

[Daniel Boelzle [:dbo]]

I don't think we should handle l10n errors like proposed in comment #0; taking it from blocking list since l10n has been fixed.

Comment 16

[Emerson Prado]

(In reply to comment #13)
> Created an attachment (id=333166) [details]
> Edit Recurrence dialog window
> 
> Emerson, do you need something like this?
> 

Yes, that's exactly what I need for pt-BR.

Comment 17

[Emerson Prado]

Removed the " de " from pt-BR localization and errors disappeared.

Comment 18

[Andreas Treumann]

This error occurs also in the es_AR build of Sunbird 20080813