Land minimal localization support

RESOLVED FIXED

Status

Add-on SDK
General
P1
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: ochameau, Assigned: ochameau)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Gandalf tried many month ago to add localization support in jetpack.
 
Our first step now is to land this `localization` module again in the SDK.
At first sight, this module still works.
But two things are required before landing this:
- Ensure that the related website works:
  https://l10n.mozillalabs.com/
  The current deployed version has not been updated for months, so there is some issues (for ex: our addon manifest changed). I filled some tickets to fix issues on commonpool project and waiting for them to land and be deployed.
There is still an opened issue: uploaded addons are not registered (see ticket #20)

- Fix startup issue mentioned ealier in bug 619807:
  Locales are not bundled with the addon but retrieved with an xmlhttprequest made on startup. Here is some way to fix this:
   * Wait for it to be done before running the addon on first launch, at least. 
   * Try to read locales files on addon startup from the XPI.

We may come up with a better solution on next iteration, 
we do not need it to be really easy to translate an addon,
I'm not convinced we even need commonpool website to be fully working.
But we have to provide stable mechanism over time and future releases.

If you want to play/follow this work, I'll host it on the following branch:
https://github.com/ochameau/addon-sdk/tree/localization

If you want to follow commonpool bugs:
https://bitbucket.org/indifex/transifex-commonpool/issues
And commonpool code:
https://bitbucket.org/indifex/transifex-commonpool/changesets
(Assignee)

Comment 1

6 years ago
Created attachment 565220 [details]
Pull request 243
Assignee: nobody → poirot.alex
Priority: -- → P1
(Assignee)

Comment 2

6 years ago
I pushed a new commit (3e111a2) that explore a complete different path than online web translation. Here is what it does:
This patch still target exactly the same addon developer API, with `_`.
But, instead of retrieving string from online service, we ship them dirrectly in the XPI.
In order to do so, our packages now support "locale" folder (like lib, data, test and doc. You can specify a custom one in your package.json). You can only have only one such folder per package (cfx currently support multiple lib/test folders).
Then, you can put multiple locale files in it. Each file is named : $(locale).json, like en-US.json, en-GB.json, fr-FR.json, ...

So a package looks like:
/my-addon/ 
  - package.json
  - lib/
    - main.js
  - locale/
    - en-US.json
    - fr-FR.json
These files are JSON files that contains an object with a `keys` attribute. This attribute is an object that act like an hashmap for all localizable strings.

/locale/fr-FR.json:
{
  "keys": {
    "Yes": "Oui",
    "No": Non",
    "Hello": "Bonjour"
  }
}
Here `keys` seems useless, but it will allow us to add new stuff to locale files.
I'm thinking about line/module specific translations.

Then cfx fetch all these files and merge them into one per language int the XPI
/my-addon.xpi/
  - install.rdf
  - resources/
  - locales/
    - en-US.json
    - fr-FR.json
These files are identic to original ones, the only thing is that we merge all their `keys` hashmap into one.

What I would have like to implement but I can't because of platform limitation:
- module and line specific translation:
Components.stack is broken when used in sandboxes.
See bug 307984 or 332176
- package specific location
Cu.getGlobalForObject is broken on wrappers Bug 613315

I've made many choices while implementing this proposal:
- there is only one locale file per language in packages. This simplify the use of localization module so that the developer doesn't have to specify one. He doesn't even have to care if there is a locale file or not. He only has to use `_()` around strings to localize.
- while building the XPI, I merge all strings. If there is a conflict I ignore it and I take to last string I get (last package wins in alphabetic name ordering). We can throw error or warning messages in order to avoid or handle this edge case of "one locale per package". Or we can fix it if we found a way to implement package specific translation.

I've made these both choices in order to be closer to the behavior of the online translation service already implemented (commonpool).

Here is a list of improvement:
- metadata translation: strings living in package.json. Addon name, description
- see these locale files are "AMO-proof"
- avoid loading all strings in memory, use platform capability like localstorage or indexedDb or ???
- simplify XPI layout in order to simplify/stabilize server code that fetch strings from the XPI. We may want to send package source instead of the XPI ...
(Assignee)

Comment 3

6 years ago
I forgot to mention that I renamed `localization` module to `l10n` in order to simplify its use. I'm convinced that "the easier it is to localize an addon, the more addon will be localizable".

Next step are:
1/ Wait to gather maximum feedback from: addon developers, localizers and tools that plays with XPI/packages (addon builder, commonpool, amo?) I expect MozCamp to be helpfull for that!
2/ While waiting, I'll try to fix commonpool and see if locale files can be handled by it.
There are several open questions with your approach:

1) When will the XPI be generated. Just before downloading it from AMO? That may be resource heavy, earlier? then we may send outdated localizations.

One approach that has been investigated was to improve Firefox download mechanism to pull l10n .json file together with an XPI when the user is installing an addon.

2) If you can keep the two levels of exceptions then we'll keep the parity with CommonPool service for further updates while using your approach as an initial one.

What I like about it is that you're aiming at resolving the outstanding issue of first-run experience.

wrt. localization vs l10n, Myk was a proponent of more descriptive naming scheme. Not sure if that has changed.

wrt. databases, I tried indexedDB approach and gave up as it increased the overhead significantly without serious benefit. I believe that we should optimize for <100 strings jetpacks and if you want to store separate DB for each jetpack then you don't benefit from shared resources at all.
(Assignee)

Comment 5

6 years ago
(In reply to Zbigniew Braniecki [:gandalf] from comment #4)
> There are several open questions with your approach:
> 
> 1) When will the XPI be generated. Just before downloading it from AMO? That
> may be resource heavy, earlier? then we may send outdated localizations.
> 
> One approach that has been investigated was to improve Firefox download
> mechanism to pull l10n .json file together with an XPI when the user is
> installing an addon.

I'm seing this current proposal as an alternative way to translate addons. 
If a developer want to build  a localizable addon *now*, he can do it 100% locally, like regular XUL addons. Then all languages are shipped into the XPI, again, like traditional addons. So it doesn't require any AMO modification. 
The addon developer is building this XPI manually and upload it to AMO.
Then we will have to plug Commonpool and AMO into this localization module.
Here is multiple way to achieve this:
 - find a way to build one xpi per language in AMO. We simply ship only one of these .json files in locales/ folder,
 - or we implement something in Addon manager that will fetch locales files from AMO before evaluating bootstrap.js
 - or we fetch locales in our bootstrap.js before evaluating main addon module.
We do not need to choose this option now, I only want to ensure that my current approach will not introduce regression or API change, when we are going to execute this move.

> 
> 2) If you can keep the two levels of exceptions then we'll keep the parity
> with CommonPool service for further updates while using your approach as an
> initial one.

What do you mean by "two levels of exceptions" ?

> 
> What I like about it is that you're aiming at resolving the outstanding
> issue of first-run experience.
> 
> wrt. localization vs l10n, Myk was a proponent of more descriptive naming
> scheme. Not sure if that has changed.
> 
> wrt. databases, I tried indexedDB approach and gave up as it increased the
> overhead significantly without serious benefit. I believe that we should
> optimize for <100 strings jetpacks and if you want to store separate DB for
> each jetpack then you don't benefit from shared resources at all.

I wasn't able to catch bent. I really want to have a platform guy advice about this. I'm convinced that there is a more efficient way of storing such hashtable.
For exemple by avoiding keeping in memory JS objects but simple C++ hashtable. And I was hoping that we can use some JSON datastorage already implement in our platform.
The cost of having yet another file to read and write is extremelly expensive in mobile world and I was highly suggested by some mobile devs to take care of that.
(Assignee)

Comment 6

6 years ago
I wrote on my blog about this bug:
   http://blog.techno-barje.fr/post/2011/10/31/jetpack-localization/

I start discussing with localizers and I changed JSON files in order to be simplier.
This blog post describe the whole picture of what I'm trying to push though this bug. I tried to justify our choices, and expose what other choices we may take.

Comment 7

6 years ago
Adding any L10n format into Mozilla stuff should be please be coordinated with Mozilla's L10n drivers. Adding Axel so he sees what's going on here.
(Assignee)

Comment 8

6 years ago
We are working closely. Gandalf started working on l10n support for jetpack.
I'm now trying to get some basic localization support into jetpack by doing the bridge between addon developers and localizer worlds!
Me, gandalf and Axel discussed last week during MozCamp about this.
We all know that one unique format would be optimal, but there is multiple things here:
 - l20n seems to be future, not present. Nothing seems to have landed in m-c, right? And we want something in jetpack soon. From my very european point of view it should have been part of jetpack 1.0 release!
 - jetpack localization rely on gettext pattern, in order to be able to automatically fetch keys to translate from source code. And current l20n format, "lol", isn't compatible with this approach as keys can't contain arbitrary string, with spaces and non-ascii characters.

Comment 9

6 years ago
(In reply to Alexandre Poirot (:ochameau) from comment #8)
>  - l20n seems to be future, not present. Nothing seems to have landed in
> m-c, right?

Right, unfortunately, but work is moving now. Bug 595821 is about the jetpack/add-on SDK parts, bug 704671 exists now to do things for HTML5 and bug 704500 has the Gecko API behind the implementations. WIP patches are there, let's see how it moves forward.

I see that you probably want an interim solution here until L20n can be fully used, so I guess it would be best to model the format in a way that it can easily be shoved into L20n in the future.
(Assignee)

Comment 10

6 years ago
I've created an etherpad in order to discuss about the final format decission:
https://jetpack.etherpad.mozilla.org/localization
What I've proposed and why. And what we should end up using.

Comment 11

6 years ago
To follow up here, we *really* should use .properties, and not try to invent a new wheel inbetween the status quo and l20n.
(Assignee)

Comment 12

6 years ago
(In reply to Axel Hecht [:Pike] from comment #11)
> To follow up here, we *really* should use .properties, and not try to invent
> a new wheel inbetween the status quo and l20n.

Message well received and addressed :)

Sumup about format choice that ended up with properties:
https://jetpack.etherpad.mozilla.org/localization

Long term workflows for localization of jetpack addons:
https://jetpack.etherpad.mozilla.org/localization-picture
(Assignee)

Comment 13

6 years ago
Comment on attachment 565220 [details]
Pull request 243

Time for review! Here is some notes to ease the review.

You may read the l10n big picture:
https://jetpack.etherpad.mozilla.org/localization-picture

A quick overview how code works with the big picture:

 # cuddlefish/packaging.py:
Read properties file from `locale` folder for each package,
merge all keys into a global hashmap: build.locales
I'm not satisfied about this, I'd like to segregate translation for each package, but I wasn't able to do it due to multiple platform issues (see middle of comment 2))

 # cuddlefish/xpi.py
Simply takes this big hashmap and serialize it to a JSON file into `locales` folder in root xpi folder. One JSON file per language.

python ===== > JS

 # addon-kit/l10n.js
Get path to root folder of the XPI from bootstrap.js that defines options.root
Read useragent locale pref and read/parse best JSON locale file, put it in `globalHash` variable which is the big hashmap of keys => values.


Possible future breakages/changes:
1. All translations are merged. I need to find a way, in JS to overload a specific translation for a particular module, package and/or line?

2. Content script l10n: 
  a) It isn't reasonnable to load the whole locale JSON file in content process (for performance/memory reason)
  b) It is complicated (impossible?) to implement gettext automatic key parsing?

3. Having more than one properties files. I don't see any usecase for now and even if we had to introduce this, it will be incremental.


1. and 2.a) can be solved by complexifying json files and storing keys for each module. And as I said in etherpad note, it will only change this internal json data format and it should not break anything!
But 2.b) makes it hard for content scripts :( And I don't have any solution in mind for the moment.
Attachment #565220 - Flags: review?(warner-bugzilla)
Your l10n.js doesn't considering about xx-yy-zz type locales.
If user's general.useragent.locale is "ja-JP-mac" and addons have ja-JP.json, we should use it but now you only search ja-JP-mac.json and ja.json.

Please support aa-bb-cc (or any longer) locale code like this:

 function searchAddonLocaleFile(preferred) {
   // Locale files are stored in `locales` folder in XPI root folder:
   let localesPath = file.join(require("@packaging").root, "locales");
   let locales = file.list(localesPath);
   let localeFile = null;
   // Select exact matching first
-  if (locales.indexOf(preferred + ".json") != -1) {
+  if (locales.indexOf(preferred + ".json") !== -1) {
     localeFile = preferred + ".json";
   }
   // Then ignore yy in "xx-yy" pattern.
   // Ex: accept "fr-FR", if `preferred` is "fr"
+  // Ex: accept "ja-JP" or "ja", if `preferred` is "ja-JP-mac"
   else {
-    let prefix = preferred.replace(/-.*$/, "");
-    for each(let filename in locales) {
-      if (filename.indexOf(prefix + "-") == 0) {
-        localeFile = filename;
-        break;
+    for (let i, prefix = preferred; (i = prefix.lastIndexOf("-")) !== -1;) {
+      prefix = prefix.substring(0, i);
+      if (locales.indexOf(prefix + ".json") !== -1) {
+         localeFile = prefix + ".json";
+         break;
       }
     }
   }
   if (!localeFile)
     return null;
 
   return file.join(localesPath, localeFile);
 }
FYI: Above code is not same algorithm as Firefox do.

Firefox will select locale file in this order:
1) the exact selected locale
2) the locale whose prefix match
   If both "xx" and "xx-yy" exists when preferred locale is "xx-yy-zz",
   Firefox just select the locale found first (Not always select "xx-yy").
3) "en-US" file if it exists
4) any available locale/theme

Above code (comment 14) don't treat "en-US" as special one since Firefox always have "en-US" and it's natural to use it for fallback but add-on's original language is not always "en-US".
# IMHO it's nice if add-on author select "default" fallback locale.

Firefox doesn't always prefer "xx-yy" than "xx" if user locale is "xx-yy-zz" but I believe Firefox should select "xx-yy". So above code always find longer locale code first.

The code to select (fallback) locale in Firefox is:
http://mxr.mozilla.org/mozilla-central/source/chrome/src/nsChromeRegistryChrome.cpp#93
http://mxr.mozilla.org/mozilla-central/source/chrome/src/nsChromeRegistryChrome.cpp#637
(Assignee)

Comment 16

6 years ago
Oh cool, Thanks for this patch.
Actually I was planning to submit a followup bug with some specific localization improvements. And language selection is one of them:
https://github.com/mozilla/addon-sdk/pull/243/files#L0R102

In the current patch, I tried to reproduced the Firefox algorithm you mentioned but I didn't realized that it was supporting xx-yy-zz! I even though that Firefox wasn't using such pattern at all.

So I agree with you, I think we should not stick with Firefox locale matching system. Having said that I do not want to slow down this very first localization support. So I opened a dedicated bug 711041 that will hopefully land soon after this one.

Comment 17

6 years ago
FTR, I'd suggest to not take the pull request, as the changesets in there are likely more confusing than helpful in understanding what the code is doing and why.

Architecturally, I don't think that shipping JSON files is right, they'll recreate all the problems we discussed previously down the road, aside from the human editing part.
(Assignee)

Comment 18

6 years ago
These JSON files are only runtime files, so that developers should never have to ever open those. The only thing they are going to manipulate is properties files living in `locale` folder, in their packages.
Just to be extra clear: we do not care about comments, nor multiline strings during runtime. So JSON is totally fine for this use.
See etherpad notes from comment 12 for more details.

Please do not block such landing because of some missing pieces.
We currently do not have ANY localization support!
Are you willing to block this landing because of unexpected locale selection ?
We will have plenty of time to fix that, but it is important to land such big piece of code so that it can bake for some time before being released.

I can integrate Tomoya's contribution in pull request if you find my "fix-it in a followup bug" approach too dangerous.

Comment 19

6 years ago
Neither of the etherpads make sense to me, for context.

Also, my concern about pulling that branch as is is not necessarily about the end-to-end diff, but about all the changesets inbetween, that go back and forth over various formats. Agressively stashing and rebasing would address that.

I consider the json data to be data off the web, and the code right now to be reliant that that data is actually generated by the source you're creating. Every feature we'll add make that more complex and brittle.
As for the format of locale files, I (Japanese locale leader from Fx 1.0) agree with Axel. We should not define our own new format for l10n unless it is really needed.

(In reply to Alexandre Poirot (:ochameau) from comment #18)
> These JSON files are only runtime files, so that developers should never
> have to ever open those. The only thing they are going to manipulate is
> properties files living in `locale` folder, in their packages.

No, even if the SDK automatically generate json from properties, not all add-ons developer use SDK. If you require SDK to l10n, all localizer need python environment but localizer don't always have that.
Some localizer will localize extensions from xpi file. That is, they extract xpi file, edit l10n files and zip them again. But in that case, they need to edit JSON files.
This is important problem since many developers will upload only xpi, not open whole original code with properties. Only with xpi files, localizer need edit json files.
# of course, including both properties/json files in the final xpi will not solve

Another example is, some add-on developers now actually use modules with SDK but without cfx tool. In that case they need to edit JSON file.

When we use two format for l10n, we need maintain code for converter every time we customize or create new tool forever. It's not happy story.

So properties/json converter will not enough solution.

One more problem is, current implementation don't support multiple properties files for a locale. This may be another bug but it's important to implement first since file/directory structure depend on it ("fix-it in a followup bug" approach doesn't work well).
Same as format, we should support same dir path as existing xul add-ons (locales/en-US/example.properties and locales/ja/example.properties)

(In reply to Axel Hecht [:Pike] from comment #19)
> I consider the json data to be data off the web, and the code right now to
> be reliant that that data is actually generated by the source you're
> creating. Every feature we'll add make that more complex and brittle.

Yes, add-ons need not always use json. Jetpack can be written with js only and JSON seems to be natural selection but in this case we already have properties and if you keep using it, supporting both is not good for some cases as I wrote above.

(In reply to Alexandre Poirot (:ochameau) from comment #18)
> Please do not block such landing because of some missing pieces.
> We currently do not have ANY localization support!

Yes, it's really important to support any l10n support ASAP.
So, here is the sample l10n module to load properties files directory:
https://bitbucket.org/knagato/add-on-sdk-l10n-module/src/default/lib/l10n.js
# sample usage:
# https://bitbucket.org/knagato/add-on-sdk-l10n-module/src/default/lib/main.js

This module is not yet perfect and coding style doesn't match for SDK but it can:
 - load properites files with StringBundle interface same as XUL extension do
 - support multiple properties
 - select default (fallback) locale

How do you think?
(Assignee)

Comment 21

6 years ago
(In reply to dynamis (Tomoya ASAI) from comment #20)
> As for the format of locale files, I (Japanese locale leader from Fx 1.0)
> agree with Axel. We should not define our own new format for l10n unless it
> is really needed.

We are suggesting here to use properties files. The ancestral l10n format.

> No, even if the SDK automatically generate json from properties, not all
> add-ons developer use SDK. If you require SDK to l10n, all localizer need
> python environment but localizer don't always have that.

1) All Jetpack/Addon-SDK developers **have to** go through `cfx` tool. Even the online tool, Addon-Builder is based on it. There is absolutly no way to create a Jetpack addon without `cfx`. So yes, you have to have python installed to create a jetpack addon, or, use AddonBuilder.

2) Please do not mix XUL Addons and Jetpack addons. Here, the topmost priority is about providing an simple addon development experience for Jetpack. So what we are suggesting to use here won't change anything for XUL Addons, they will stick to properties and dtd. Jetpack and XUL addons are two distinct things and this l10n module wont be use for XUL addons!

> Some localizer will localize extensions from xpi file. That is, they extract
> xpi file, edit l10n files and zip them again. But in that case, they need to
> edit JSON files.
> This is important problem since many developers will upload only xpi, not
> open whole original code with properties. Only with xpi files, localizer
> need edit json files.
> # of course, including both properties/json files in the final xpi will not
> solve

This is something I am aware of. We currently have similar issue with the repacking service and in both cases, upload an XPI is sub-optimal for multiple reason in Jetpack case. So that we will have to upload something else than an XPI. 
Again, do not consider that these things will apply to XUL addons. All this stuff is going to be Jetpack-specific.

> Another example is, some add-on developers now actually use modules with SDK
> but without cfx tool. In that case they need to edit JSON file.

There is no way to create a jetpack addon without using cfx.

> When we use two format for l10n, we need maintain code for converter every
> time we customize or create new tool forever. It's not happy story.

Again, only cfx will manipulate these JSON files. So that if you want to regenerate a jetpack addon with new/updated locale, you won't have any other choice than using cfx!

> One more problem is, current implementation don't support multiple
> properties files for a locale. This may be another bug but it's important to
> implement first since file/directory structure depend on it ("fix-it in a
> followup bug" approach doesn't work well).

Sure. "Fix it in a followup bug" doesn't work well with everything. 
The bug I've suggested to fix later is way less disruptive!
Please try being more positive. That's not because I suggested that one thing should be fixed right after, that I'm considering that this patch is perfect and nothing can't be changed in it.
I understand that one locale file per language is very limited.
I choosed to use only one to simplify localization story from the developer perspective, but I may be wrong on this.

gozala/myk: It is not very clear to me how we should handle localization files in the future of packageless jetpack. Do you have any thought on that topic ? Should we use one localization file per module or per package ?
My main concern is about code sharing. One file per package sounds bad for a packageless world!
I don't think we can easily match each module to one localization file by using the file system. And we should not require developers to create/specify a specific localization file, as they won't need any such file if they are using gettext pattern. We can still offer this feature optionally!

> Same as format, we should support same dir path as existing xul add-ons
> (locales/en-US/example.properties and locales/ja/example.properties)

XUL Addons is another thing. We do not use chrome.manifest nor content/locale/skin/components directories at all. The whole files layout has nothing to do with XUL addons and XPI are not meant to be edited!

> Yes, add-ons need not always use json. Jetpack can be written with js only
> and JSON seems to be natural selection but in this case we already have
> properties and if you keep using it, supporting both is not good for some
> cases as I wrote above.

Runtime code, l10n.js module will only receive JSON files coming from the XPI or CommonPool web service.

> Yes, it's really important to support any l10n support ASAP.
> So, here is the sample l10n module to load properties files directory:
> https://bitbucket.org/knagato/add-on-sdk-l10n-module/src/default/lib/l10n.js
> This module is not yet perfect and coding style doesn't match for SDK but it
> can:
>  - load properites files with StringBundle interface same as XUL extension do
>  - support multiple properties
>  - select default (fallback) locale
> 
> How do you think?

I think that this code is just sticking with old XUL pattern without simplifying anything for Addon development and this isn't what we are trying to achieve.
Here I'm trying to stick to gandalf's original project that will offer a gettext pattern, where, for simplest addons, you won't even have to create any localization file. Instead, keys will be automatically fetch by `cfx` in order to build localization file templates (like .po files).
(Assignee)

Comment 22

6 years ago
(In reply to Axel Hecht [:Pike] from comment #19)
> Also, my concern about pulling that branch as is is not necessarily about
> the end-to-end diff, but about all the changesets inbetween, that go back
> and forth over various formats. Agressively stashing and rebasing would
> address that.

I've pushed a new branch with only one commit.
Sorry about long posts here. I don't intend to block, just wanted to make sure everything considered enough.

In short:
I still don't understand well why you really need to introduce new format for 'minimal' l10n support if you already aware of the issue about repackaging. But I think it's better than no l10n support for a long time (already too long after sdk 1.0).

(In reply to Alexandre Poirot (:ochameau) from comment #21)
> 1) All Jetpack/Addon-SDK developers **have to** go through `cfx` tool. ...

Yes, I know Jetpack developers must always go through cfx tool.
Just to make it clear, what I meant was, bootstrapped restartless extensions written by hand but using some modules from sdk. It's edge case and need not care too much, sorry.

> > Some localizer will localize extensions from xpi file. That is, they extract
> > xpi file, edit l10n files and zip them again. But in that case, they need to
> > edit JSON files.
...
> This is something I am aware of. We currently have similar issue with the
> repacking service and in both cases, upload an XPI is sub-optimal for
> multiple reason in Jetpack case. So that we will have to upload something
> else than an XPI. 

This is the problem I'm worrying about most since it's not just technical problem but l10n eco system one. Simpler system make more l10n contribution. New format/tool will make existing contributers leave.
AMO may be able to ask developers to upload not only xpi but cannot force it. There are not a few self-hosted add-ons too. It's difficult to solve this problem.

Even when localizer translate json file first, if add-on author can convert back to properties with cfx easily, it's easy to merge the contribution with smallest costs.
If you also provide bi-direction converter which will not require python (exe or converter on the web?), it's a little messy but localizer can use existing tools.

> > One more problem is, current implementation don't support multiple
> > properties files for a locale. This may be another bug but it's important to
> > implement first since file/directory structure depend on it.
...
> I understand that one locale file per language is very limited.
> I choosed to use only one to simplify localization story from the developer
> perspective, but I may be wrong on this.

If you will continue supporting one file per xpi style even after you support multiple files, we can go with current api. In this case keep current api as an easiest (default) way of l10n module and add optional api for multiple files like this:
  A. l10n.get(key) will load locales/<locale>.json (same as current your l10n.js).
 B1. l10n.get(key, path) will load locales/<locale>/<path>.json
 B2. l10n.get(path+"."+key) will load locales/<locale>/<path>.json
 B3. get=l10n.load(path); get(key) will load locales/<locale>/<path>.json

I don't like B1/B2 style since we always need to specify the path. There will be too many lines which depends on the locale file path.
I prefer B3 style since we can write simpler and only 1 line depends on the path.

> gozala/myk: It is not very clear to me how we should handle localization
> files in the future of packageless jetpack. Do you have any thought on that
> topic ? Should we use one localization file per module or per package ?
> My main concern is about code sharing. One file per package sounds bad for a
> packageless world!

Some module provides user interface and/or log messages. I think l10n file should be defined at least per module. Better to be able to use multiple files for each modules.

> I think that this code is just sticking with old XUL pattern without
> simplifying anything for Addon development and this isn't what we are trying
> to achieve.

I think providing the same way for existing xul developers is also useful for easier migration from xul to jetpack. But actually it's not easy to migrate whole code and I can understand we need not care too much about backward compatibility with xul one.
If new format is needed to achive better l10n system we shoud define it (though I'm not sure why you need new format for what).

> Here I'm trying to stick to gandalf's original project that will offer a
> gettext pattern, where, for simplest addons, you won't even have to create
> any localization file. Instead, keys will be automatically fetch by `cfx` in
> order to build localization file templates (like .po files).

Though this may be off-topic, one concern about gettext style (common pool?) is, if the key is always "text label" (usually English text label), we cannot select the translation depending on the context. Translation is not one-to-one work. Localizers often have trouble with this localizability problem. It's really annoying and limit us to translate perfectly.

Even when original word (in English) is same, translation should be different. If the key is "id name" like Firefox, we can define different entities for different context. Even when original language need minor update like typo fix, the key will not change and any other l10n file need not updated.
At the same time, this can treat each language equally (English is not always special).

I hope we'll go latter way to make us possible to do better translation without localizability problem.
@dynamis: just a comment on the patch that you provided. I believe that we (Pike, Alexandre and me) agreed to go for string2string localization as the initial one (gettext like approach where a source string is the ID) and on the dev front replicate gettext-like environment where you wrap source strings in _() in order to have them localizable.

Your patch is addressing the other approach where the developer introduces strings in a separate file by hand and assigns artificial IDs to them (the behavior we now have in .properties and .dtd).

Those two approaches are complementary and the vision we had for jetpack l10n is to first go with the gettext-like behavior (see my original patch for common pool) and then add L20n as teh more sophisticated approach.

If we want to use .properties for storing data in the gettext-like approach, then let's do this, but let's not alter the workflow in this approach to mimic the other one that will come with l20n support in Jetpack.
Comment on attachment 565220 [details]
Pull request 243

I added a bunch of comments to the pull request. There's one or two that need fixing for sure, but otherwise I'm r+.
Attachment #565220 - Flags: review?(warner-bugzilla) → review+

Comment 26

6 years ago
Commit pushed to https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/98976787e30b2cb0384ca6004edf58e23b3f6199
Merge pull request #243 from ochameau/localization

Bug 691782: Land minimal localization support r=@warner
Landed! And then https://github.com/mozilla/addon-sdk/commit/1d4475e92da1ea49ece960182c8b11d6791ee0d9 avoided breaking it! So we can close this one.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 28

5 years ago
A quick update that may interest cc-ed people.

I promise to iterate on l10n implementation, so we have 3 followup bugs opened:
 - bug 711041: improve language matching algorithm
 - bug 719399: add "default locale" setting
 - bug 719402: implements real plural forms
You need to log in before you can comment on or make changes to this bug.