If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

The calendar widget is not localizable

RESOLVED FIXED in Bugzilla 4.4

Status

()

Bugzilla
User Interface
--
minor
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: Wurblzap, Assigned: Wurblzap)

Tracking

3.7.2
Bugzilla 4.4
Bug Flags:
approval +
blocking4.0 -

Details

Attachments

(1 attachment, 2 obsolete attachments)

5.75 KB, patch
Frédéric Buclin
: review+
Details | Diff | Splinter Review
(Assignee)

Description

7 years ago
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.
Flags: blocking4.0?

Comment 1

7 years ago
IMO not a blocker. It's much less critical than e.g. bug 215210, and that one is not a blocker either.
(Assignee)

Comment 2

7 years ago
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

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

Comment 4

7 years ago
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

7 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: blocking4.0? → blocking4.0-
Resolution: --- → WORKSFORME

Updated

7 years ago
Target Milestone: Bugzilla 4.0 → ---
(Assignee)

Comment 6

7 years ago
Created attachment 464950 [details] [diff] [review]
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.
Assignee: create-and-change → wurblzap
Status: RESOLVED → REOPENED
Attachment #464950 - Flags: review?(mkanat)
Resolution: WORKSFORME → ---

Comment 7

7 years ago
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?
Attachment #464950 - Flags: review?(mkanat) → review-
(Assignee)

Comment 8

7 years ago
(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.
Status: REOPENED → ASSIGNED

Comment 9

7 years ago
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.
(Assignee)

Comment 10

6 years ago
Created attachment 615052 [details] [diff] [review]
Patch 2

Here's a solution with less impact.

@hakon: Is there already a bug for the comments.js thing?
Attachment #464950 - Attachment is obsolete: true
Attachment #615052 - Flags: review?(LpSolit)
(Assignee)

Updated

6 years ago
Target Milestone: --- → Bugzilla 4.2
(Assignee)

Comment 11

6 years ago
(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.
(Assignee)

Comment 12

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

Comment 13

6 years ago
(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.
(Assignee)

Comment 14

6 years ago
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

6 years ago
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).
Severity: normal → minor

Comment 16

5 years ago
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.
Attachment #615052 - Flags: review?(LpSolit) → review-

Comment 17

5 years ago
I don't want to add a new template in a stable release.
Component: Creating/Changing Bugs → User Interface
Target Milestone: Bugzilla 4.2 → Bugzilla 4.4
(Assignee)

Comment 18

5 years ago
Created attachment 625064 [details] [diff] [review]
Patch 2.1

Agreed :)
Here's an updated patch.
Attachment #615052 - Attachment is obsolete: true
Attachment #625064 - Flags: review?(LpSolit)

Comment 19

5 years ago
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
Attachment #625064 - Flags: review?(LpSolit) → review+

Updated

5 years ago
Flags: approval+
(Assignee)

Comment 20

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

Committing to: bzr+ssh://wurblzap%40gmail.com@bzr.mozilla.org/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.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.