554205 - [k] Localize javascript

Status: VERIFIED FIXED

(support.mozilla.org :: General, defect, P4)

Description

[Paul Craciunoiu [:paulc]]

We should take advantage of AMO's localization work and also localize javascript. This summarizes how:
http://jbalogh.github.com/zamboni/#gettext-in-javascript

Comment 1

[James Socol [:jsocol, :james]]

Pushing out low-priority bugs.

Comment 2

[Josh Gross (:jgross)]

Followed :jbalogh's instructions, added gettext() to appropriate Javascript.

:stas -- is this the right thing to do?

Comment 3

[Staś Małolepszy :stas]

Yes, I think so :)

There's more to it, however. I think it would make sense (similar to AMO) to keep the javascript strings in a separate file, so that we only send this small file to the client, instead of the whole messages.po with all Kitsune strings.

If the javascript strings are kept in separate files, we'll need a process for managing these files.

AMO uses the Tower library written by clouserw to merge extracted POT file with existing PO files. The command it uses is `./manage.py amalgamate`. It's rather AMO-specific.

In Kitsune, we're using http://github.com/jsocol/sumo-locales/blob/master/merge-po.sh instead. It's not adapted to handling multiple PO files, but could be modified accordingly. Or, we could try reusing parts of `./manage.py amalgamate` for something Kitsune-specific (e.g. taking bug 564831 into advantage).

If you think that's a good idea, I can try cooking something up.

(CC'ing Wil)

Comment 4

[Josh Gross (:jgross)]

Attachment: Adds extract-po.py and extracted javascript.po and .mo

I actually adapted AMO's "extract-po.py" script, so it creates a separate javascript.po/mo files containing only the Javascript strings.

Attached is a proposed patch adding the appropriate files.

Comment 5

[Josh Gross (:jgross)]

Attachment: Correct patch file

Whoops, here is the correct patchfile.

Comment 6

[Staś Małolepszy :stas]

This patch handles the extraction part (which we were doing via tower's `extract` command), but it doesn't handle the merging part.

Every time you extract strings from the code into a POT file, you'll need to merge them somehow with existing translations in existing PO files. Tower's `amalgamate` command does this, but in an AMO (or Zamboni) specific way. I can work on adapting this to Kitsune on Friday. By adapting I mean removing the logic handling old PHP translations from Remora (r-keys, z-keys and such) as well as adding the fix from bug 564831.

Here's a link to Tower: http://github.com/clouserw/tower, btw.

Comment 7

[Staś Małolepszy :stas]

Working on this now, I'll post an update later today.

Comment 8

[Staś Małolepszy :stas]

I think we should extend Tower rather than creating another script. So, I forked it and added 2 new commands: merge and verbatimize. The code is here: http://github.com/stasm/tower/commit/a536b385f9c0717bf479c1a04ed4624d1389a652

Wil, can you please take a look at this and let me know what you think? In the long run, I'd like merge to replace amalgamate (once we abandon remora completely).

I'll also attach a patch momentarily against Kitsune, which allows for localization of javascript.

Comment 9

[Staś Małolepszy :stas]

Attachment: Patch A. Enable javascript l10n

I'm not sure about this line:

+<script src="{{ url('jsi18n') }}"></script>

Can we add this to a bundle instead? 

Also, Zamboni uses the following line:
<script src="{{ url('jsi18n') }}/build:{{ BUILD_ID_JS }}"></script> in http://github.com/jbalogh/zamboni/blob/master/templates/base.html#L83

I'm not sure what /build does, so I left it out.

Comment 10

[Staś Małolepszy :stas]

Attachment: Patch B. CHanges to the process as documented in README.txt

This is how the new process would look like.

Comment 11

[Jeff Balogh (:jbalogh)]

(In reply to comment #9)
> Created an attachment (id=451657) [details]
> Patch A. Enable javascript l10n 
> 
> I'm not sure about this line:
> 
> +<script src="{{ url('jsi18n') }}"></script>
> 
> Can we add this to a bundle instead? 

Right now that returns a different catalog for each locale.  It may be better to trade extra bytes for 1 less request, but I haven't looked into that yet.  Since all the catalogs are dynamically generated, we'd have to extend the minify script to extract all the catalogs.
 
> Also, Zamboni uses the following line:
> <script src="{{ url('jsi18n') }}/build:{{ BUILD_ID_JS }}"></script> in
> http://github.com/jbalogh/zamboni/blob/master/templates/base.html#L83

The build part lets us front-end cache the jsi18n path for a year (and we cache the function output with @cache_page).  When we update js we invalidate the old path by getting a new build id.

Comment 12

[Staś Małolepszy :stas]

(In reply to comment #8)
> I think we should extend Tower rather than creating another script. So, I
> forked it and added 2 new commands: merge and verbatimize. The code is here:
> http://github.com/stasm/tower/commit/a536b385f9c0717bf479c1a04ed4624d1389a652

BTW. the new commands assume that http://viewvc.svn.mozilla.org/vc/projects/sumo/locales/tiki/ is called "compendia". (This is so that the code is forward-compatible with other projects where we might use compendia.)

Comment 13

[Staś Małolepszy :stas]

(In reply to comment #11)
> > Also, Zamboni uses the following line:
> > <script src="{{ url('jsi18n') }}/build:{{ BUILD_ID_JS }}"></script> in
> > http://github.com/jbalogh/zamboni/blob/master/templates/base.html#L83
> 
> The build part lets us front-end cache the jsi18n path for a year (and we cache
> the function output with @cache_page).  When we update js we invalidate the old
> path by getting a new build id.

Ah, I get it, thanks for the explanation!

Comment 14

[James Socol [:jsocol, :james]]

(In reply to comment #11)
> > Also, Zamboni uses the following line:
> > <script src="{{ url('jsi18n') }}/build:{{ BUILD_ID_JS }}"></script> in
> > http://github.com/jbalogh/zamboni/blob/master/templates/base.html#L83
> 
> The build part lets us front-end cache the jsi18n path for a year (and we cache
> the function output with @cache_page).  When we update js we invalidate the old
> path by getting a new build id.

Josh, we should use this as well.

Stas, I'm a little confused. Does Patch A (attachment 451657 [details] [diff] [review]) supersede Josh's patch?

Comment 15

[Staś Małolepszy :stas]

(In reply to comment #14)
> Stas, I'm a little confused. Does Patch A (attachment 451657 [details] [diff] [review]) supersede Josh's patch?

It obsoletes it in fact. I didn't want to mark it as such without consulting it with Josh and you first.

Comment 16

[Staś Małolepszy :stas]

To elaborate some more: you'll need both patch A and Tower from http://github.com/stasm/tower to make this work. Plus, "tiki" renamed to "compendia" in locale/.

I'll be on a plane to MV tomorrow, so if you wish we can discuss tomorrow irl :)

Comment 17

[Staś Małolepszy :stas]

Attachment: Patch A2. Updated patch A to tip, removed cache_page decorator for jsi18n url

Here's an updated patch to enable javascript localization in Kitsune. The previous one wouldn't apply anymore due to changes in media/js/ and some other files.

There's also one more important change. In the previous patch I did the following:

+    # Javascript translations.
+    url('^jsi18n/.*$', cache_page(60 * 60 * 24 * 365)(javascript_catalog),
+        {'domain': 'javascript', 'packages': ['kitsune']}, name='jsi18n'),

I think that using the cache_page decorator here without using the BUILD_JS_ID in <script src="{{ url('jsi18n') }}"></script> is wrong (see comment 11). It gives us no way to invalidate the cached js file. I removed the decorator for now.

Comment 18

[Staś Małolepszy :stas]

Attachment: Patch A2. Updated patch A to tip, removed cache_page decorator for jsi18n url

Oops, sorry, attached a patch file with color definitions in it :(

Comment 19

[James Socol [:jsocol, :james]]

http://github.com/jsocol/kitsune/commit/eecf2190983c7

Comment 20

[Staś Małolepszy :stas]

I renamed "tiki" to "compendia" in SVN and applied patch B to README.txt in r69896.

Thanks!