581365 - The calendar widget is not localizable

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[Marc Schumann [:Wurblzap]]

The calendar widget is not localizable: the month and day-of-week names are in js/yui/calendar/calendar-min.js and js/yui/datasource/datasource-min.js.

Comment 1

[Frédéric Buclin]

IMO not a blocker. It's much less critical than e.g. bug 215210, and that one is not a blocker either.

Comment 2

[Marc Schumann [:Wurblzap]]

I agree that it is much less critical than bug 215210.
We don't want to have things get worse than it is, though, by adding new unlocalizable things, do we?

Comment 3

[Frédéric Buclin]

Well, in this case, it worths it as having an english-only widget is much better than no widget at all.

Comment 4

[Max Kanat-Alexander]

If you look at a fully expanded version of calendar.js (not the -min version), you'll see there's a Calendar.DEFAULT_CONFIG object, where you can localize the names of everything, I'm pretty sure. There are also some LOCALE constants in there that can probably be used to fix things up in some way.

Comment 5

[Max Kanat-Alexander]

Here's the recommended way to do l10n on the Calendar widget:

http://developer.yahoo.com/yui/calendar/#internationalization

If you need any code modifications in order to be able to do that, feel free to reopen this bug.

Comment 6

[Marc Schumann [:Wurblzap]]

Attachment: Patch

Ok, this works really well.

I'm not sure about using page.cgi for js, especially seeing that it doesn't expire properly. But maybe we can iron this out.

Comment 7

[Max Kanat-Alexander]

Comment on attachment 464950 [details] [diff] [review]
Patch

Hmm. This isn't necessary for English. Is there a way to make it not take effect, or possibly to just make it some guideline for localizers instead of a code change here?

Does using page.cgi for js actually work with the new system that adds "?" to JS URLs?

Comment 8

[Marc Schumann [:Wurblzap]]

(In reply to comment #7)
> Hmm. This isn't necessary for English. Is there a way to make it not take
> effect, or possibly to just make it some guideline for localizers instead of a
> code change here?

Well, why? There's a whole lot we do that isn't necessary for English.
If you're thinking js performance, then I don't think there's an impact -- if we don't specify things, YUI will pull some defaults out of its pockets and do the same thing.

If we manage to expire this js file, I don't think we'll see an impact at all.

> Does using page.cgi for js actually work with the new system that adds "?" to
> JS URLs?

No, it doesn't. That's why I modified header.html.tmpl.
As said, the expiry thing needs work. I need help with this, though.

Comment 9

[mailtrader]

Hope this will be localizable in the final release. In general js features or code changes should be checked in languages other than English. For instance collapsing/expanding comments by using [-]/[+] is moved to js/comments.js and therefore not localizable by tmpl files.

Comment 10

[Marc Schumann [:Wurblzap]]

Attachment: Patch 2

Here's a solution with less impact.

@hakon: Is there already a bug for the comments.js thing?

Comment 11

[Marc Schumann [:Wurblzap]]

(In reply to hakon from comment #9)
> collapsing/expanding comments by using [-]/[+] is moved to
> js/comments.js and therefore not localizable by tmpl files.

Fixed in bug 745460.

Comment 12

[Marc Schumann [:Wurblzap]]

Frédéric, any chance to see this in 4.3.1?

Comment 13

[Frédéric Buclin]

(In reply to Marc Schumann [:Wurblzap] from comment #12)
> Frédéric, any chance to see this in 4.3.1?

Too late for 4.3.1. We released it a few hours ago. That's why I did no reviews recently. I was busy enough with the release stuff. :) And I'm not sure your patch is the correct way to fix the problem. I don't understand why a separate template would fix the problem.

Comment 14

[Marc Schumann [:Wurblzap]]

Ok, I see.

The idea of the patch is make life easy for all parties involved.
As a developer, I just need to PROCESS the new template.
As a localizer, I just need to modify the new template for my language, centrally.

Of course, technically, it would suffice to just extend the extend the createCalendar call in js/field.js. But as a localizer, I don't want to analyze Javascript code. In the case of the calendar, I actually need to put different Javascript code in place in order to have a correct localization. We need to make this obvious; otherwise, every newly added call to createCalendar in the future is likely to get overseen by localization efforts.

Comment 15

[Frédéric Buclin]

So if I understand comment 14 correctly, the current code in template/en/default/global/create-calendar.js.tmpl must still be edited by a localizer to get localized strings, right? If yes, how should it look like? Maybe an example should be written in the template itself for clarity (because it's currently very nebulous to me).

Comment 16

[Frédéric Buclin]

Comment on attachment 615052 [details] [diff] [review]
Patch 2

>=== added file 'template/en/default/global/create-calendar.js.tmpl'

I would prefer this template to be named global/calendar.js.tmpl.


>+[%# This template exists because createCalendar accepts additional parameters
>+  # which allow for localization. Please see js/fields.js and the YUI
>+  # documentation for details.

This comment should contain a link to the YUI documentation, e.g. to http://developer.yahoo.com/yui/examples/calendar/germany.html, as well as the order in which to pass the arguments to createCalendar(), so that the localizer doesn't have to look at js/field.js.



>=== modified file 'template/en/default/search/field.html.tmpl'

>+        [%+ PROCESS "global/create-calendar.js.tmpl" id = $field.name %]
>+        [% PROCESS "global/create-calendar.js.tmpl" id = $field.name _ 'to' %]

They generate a JavaScript error. You must write id = field.name (without $).


Otherwise your patch looks good and works fine.

Comment 17

[Frédéric Buclin]

I don't want to add a new template in a stable release.

Comment 18

[Marc Schumann [:Wurblzap]]

Attachment: Patch 2.1

Agreed :)
Here's an updated patch.

Comment 19

[Frédéric Buclin]

Comment on attachment 625064 [details] [diff] [review]
Patch 2.1

>=== modified file 'js/field.js'

>+ * using template/en/default/global/calendar.js, so that localization works.

The extension of the file is .js.tmpl, not .js. This can be fixed on checkin. r=LpSolit

Comment 20

[Marc Schumann [:Wurblzap]]

Ok, fixed that on checkin. Thanks for the review!

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified js/field.js
modified template/en/default/bug/field.html.tmpl
modified template/en/default/bug/summarize-time.html.tmpl
added template/en/default/global/calendar.js.tmpl
modified template/en/default/search/field.html.tmpl
modified template/en/default/search/form.html.tmpl
Committed revision 8232.