Status

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: zbraniecki, Assigned: zbraniecki)

Tracking

(Blocks 3 bugs)

Details

(Whiteboard: [gecko-l20n], )

User Story

L20n in Firefox will use a custom resource registry that will combine resources from .jar files and overrides with resources coming from live-updates (like kinto etc.).

We need to design and implement the registry and make l20n HTML/XUL bindings use it for resource fetching.

This is intended to let us not use ChromeRegistry for localization resources.

Attachments

(1 attachment)

58 bytes, text/x-review-board-request
zbraniecki
: review+
Details
No description provided.
User Story: (updated)
Blocks: 1280680
Blocks: 1280687
Blocks: 1280689
Blocks: 1280670
Blocks: 1280669
Blocks: 1280673
This is the very first stepping stone that we have to implement in Gecko to get L20n. It also is the one with the highest question mark factor, so I'd like to start tackling it now.

My initial approach is to build a singleton JSM Registry module that allows Sources to be plugged into it.

The Sources may look like this:

 - toolkit bundle Source
 - browser bundle Source
 - pl langpack Source
 - live-update Source

The app will then add Sources to the Registry and prioritize them. It means that the same resource ID may be registered in toolkit and browser (allowing for overrides), or in browser and live-update allowing for live updates.

The Registry API has the following methods:

L20nRegistry.getResources(requestedLangs, resIds) -> { availableLangs, resourceBundles }

L20nRegistry.registerSource(Source);

In order to start, I need to write the `registerSource` method and the first Source (File/Jar) that will allow the Registry to load sources from the app itself.

We decided to avoid Chrome Registry, so I'm going to use nsIZipReader directly.

This allows me to freely choose the resource ID scheme. My proposal is to do "package:path" style, for example: "browser:menu/menu.ftl".

The package part will allow us to combine groups of resources under common component that may have separate language negotiation strategy (devtools in en-US, browser in pl) and overrides: browser bundle Source may override toolkit's files.

Internally, the FileSource will have to define the path template inside the zip package. For example browser Source may have sth like this: "{package}/locales/{locale}/{path}" which when given "browser:menu/menu.ftl" and "en-US" will attempt to load the file from the browser zip package using path: "browser/locales/en-US/menu/menu.ftl".

Questions:

1) Does it sound like a sane strategy?
2) How do we map files into zip files?

I seem to see omni.ja file in my Firefox bundle and it has "chrome/en-US/locale/browser/aboutDialog.dtd" file.

I'd like this file to become "browser/locales/en-US/aboutDialog.dtd". Can I do this?

And is there always omni.ja? Or is there sometimes browser.jar? Or sometimes there is no jar and I have to write the whole logic to check if I need to load from zip file, or from directory?

:bsmedberg, can you advise me?
Assignee: nobody → gandalf
Flags: needinfo?(benjamin)
I'd put the path mapping to actual paths into the source object, not the registry.

For packaging and build, I suspect it's going to be easier to start with really different paths, that don't smell like chrome registry, inside the omni.ja. /localization/... might be one.

I'd also put locale code up front, and make the locale be part of the registration process. I.e., this Source for this lang:
L10nService.register(new Source..., 'en-US')

Re reading from omni.ja, you can use the jar:// protocol, I suspect. No reason to use zipreader?
> I'd put the path mapping to actual paths into the source object, not the registry.

Yes, that's my plan!

> I'd also put locale code up front, and make the locale be part of the registration process. I.e., this Source for this lang:
L10nService.register(new Source..., 'en-US')

Can a source be multi-lingual? For example, omni.jar may have multiple languages? Langpack may have multiple languages? Live-Update Source will have "all" languages :)

> Re reading from omni.ja, you can use the jar:// protocol, I suspect. No reason to use zipreader?

Can I? Will it do the directory vs zip guessing for me?
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #3)
> > I'd also put locale code up front, and make the locale be part of the registration process. I.e., this Source for this lang:
> L10nService.register(new Source..., 'en-US')
> 
> Can a source be multi-lingual? For example, omni.jar may have multiple
> languages? Langpack may have multiple languages? Live-Update Source will
> have "all" languages :)

I expect that we'll have only a few languages in each breed of shipping per user, and that's why I think it's going to be more straightforward to announce the supported language of a source at registration time.

> > Re reading from omni.ja, you can use the jar:// protocol, I suspect. No reason to use zipreader?
> 
> Can I? Will it do the directory vs zip guessing for me?

The source knows if it's a zip or not, so no guessing involved.
Follow up question for :bsmedberg:

I see "chrome/en-US/locale/browser/aboutDialog.dtd" in my omni.ja.

How do I load this file from inside of my JSM module? Should I use XHR? What's the path? I see sth like "jar:resource://gre/chrome.browser.jar!/chrome/en-US/locale/browser/aboutDialog.dtd", but not sure how am I supposed to get the "gre/chrome.browser.jar!" part and how to load it.
I don't have the ability to provide detailed feedback on the code, but general guidance (including some things I said on IRC):

* don't troll through files or directories at startup. That will always cause startup performance regressions. Instead use some system (perhaps the existing registration manifests?) to register files from known locations.
* The manifests solve your problem of not knowing whether we're in an omnijar or not: they would just pass a URL to L10nService.register
* The question of whether you should use XHR or something else probably depends on how you'll be processing the data. If you're processing in JS, and especially if you're using TypedArrays/Blob, then XHR is the obvious choice.
Flags: needinfo?(benjamin)
Comment on attachment 8776600 [details]
Bug 1280671 - Add L10nRegistry.

Hey Zibi -- here's what I used to make my changes in bug 1288406 work.  This is essentially https://github.com/zbraniecki/gecko-dev/blob/l20n-zibi/toolkit/modules/L10nRegistryNoIndex.jsm renamed to L10nRegistry and without 'en-US' hardcoded in Source's base paths.

Do you feel like this is close to what you'd like to land on larch?  How can I help?
Attachment #8776600 - Flags: feedback?(gandalf)
Comment on attachment 8776600 [details]
Bug 1280671 - Add L10nRegistry.

Yeah, that looks good. I'm still working on several small things that could potentially improve performance, but I can land them incrementally as updates.
Attachment #8776600 - Flags: feedback?(gandalf) → feedback+
Comment on attachment 8776600 [details]
Bug 1280671 - Add L10nRegistry.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68316/diff/1-2/
Attachment #8776600 - Attachment description: Bug 1280671 - Add L10nRegistry → Bug 1280671 - Add L10nRegistry.
Attachment #8776600 - Flags: feedback+ → review?(gandalf)
https://reviewboard.mozilla.org/r/68314/#review65514

Zibi, I made two small edits and I think we can land this now and then work on improvements in next iterations.  Please consult https://reviewboard.mozilla.org/r/68314/diff/1-2/ for the view of the chnages I made since your f+.  Thanks!

::: toolkit/modules/L10nRegistry.jsm:103
(Diff revisions 1 - 2)
>  }
>  
>  function fetchFirstBundle(bundles) {
> +  // worst-case scenario: the list of bundles to return to the client is empty
> +  if (bundles.length === 0) {
> +    return bundles;

I added to protect against scenarios when L10nRegistry finds no resources at all.  We'll still need to handle the empty list of bundles which is returned here on the clientside.  I think we can repurpose bug 908780 for this.
The L10nRegistry landed on larch according to the design decisions made in bug 1288665.  Please file new bugs for improvements.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Summary: Implement L20nRegistry → Implement L10nRegistry
Mass change dependency tree for bug 1279002 into a whiteboard keyword.
No longer blocks: gecko-l20n
Whiteboard: [gecko-l20n]
No longer blocks: 1280668
You need to log in before you can comment on or make changes to this bug.