Closed Bug 1101632 Opened 10 years ago Closed 9 years ago

Make it possible to produce JSON files for pseudolocales

Categories

(Firefox OS Graveyard :: Gaia::L10n, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::L10n
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: stas, Unassigned)

References

Details

Attachments

(1 file)

[PullReq] stasm:1101632-buildtime-qps to mozilla-b2g:master
46 bytes, text/x-github-pull-request
zbraniecki
: review+
rickychien
: review+
Details | Review 
Even if pseudolocales are a runtime option, it should be also possible to optionally build them on build-time to improve performance and provide closer-to-reality testing conditions (e.g. manifest files).
Depends on: 1068382
Just FYI, I'm against adding qps-ploc/plocm to app's <meta> and manifest. I believe that since those locales are not stored in the source repository, but generated at build time, they should be added at build time as well.
I'm torn on this one.  Your argument makes sense, but I could also lean the opposite way:  if it's listed in shared/resources/languages.json, it should also be listed in metas and the manifests.

Suppose we go the way of not including it:  do you have any ideas about what to do with the app names in the manifests?
Flags: needinfo?(gandalf)
I forgot to add:  I agree that it makes more sense not to add it for consistency with DEBUG=1 profiles and the case of running Gaia in Firefox.  There's no build step in this case so the buildtime pseudolocale should not be available.
(In reply to Staś Małolepszy :stas from comment #3)
> I'm torn on this one.  Your argument makes sense, but I could also lean the
> opposite way:  if it's listed in shared/resources/languages.json, it should
> also be listed in metas and the manifests.

It should not be listed in languages.json. It should be enabled by the flag during build time imo.

> Suppose we go the way of not including it:  do you have any ideas about what
> to do with the app names in the manifests?

Sure. It should be injected into the manifest at build time using pseudolocalized default locale name/desc.

For third-party apps, we should not provide translation.
Flags: needinfo?(gandalf)
(In reply to Zibi Braniecki [:gandalf] from comment #5)
> It should not be listed in languages.json. It should be enabled by the flag
> during build time imo.

I don't think I agree here. Adding build flags is hard. We don't want the flag to be turned on by default because we don't want the build time pseudolocale in production builds. OTOH we do want the pseudolocale for all developer purposes. Adding it to languages.json is the closest thing we can do to imitate a real locale (like ar or fr today). 

> 
> > Suppose we go the way of not including it:  do you have any ideas about what
> > to do with the app names in the manifests?
> 
> Sure. It should be injected into the manifest at build time using
> pseudolocalized default locale name/desc.

Well yeah, I guess I was asking about the how. Is there a way to include the pseudolocale code in multilocale.js?  Or, do I need to duplicate that code between  build/l10n.js and multilocale.js?
Depends on: 1129926
(In reply to Staś Małolepszy :stas from comment #6)

> Well yeah, I guess I was asking about the how. Is there a way to include the
> pseudolocale code in multilocale.js?  Or, do I need to duplicate that code
> between  build/l10n.js and multilocale.js?

I started working on refactoring shared/js/l10n.js and build/l10n.js such that you can require() them from our build scripts.  It's not impossible, but the changes go deep into webapp-optimize, so I filed bug 1129926 to handle this later on.
Comment on attachment 8559178 [details] [review]
[PullReq] stasm:1101632-buildtime-qps to mozilla-b2g:master

Zibi, can you take a look at this approach?

I had to copy some of the pseudol10n code so that I could require it from multilocale.js and from webapp-optimize.js.  I put it in a separate file, build/l10n-pseudo.

Once bug 1129926 is fixed, we can remove this file and require('shared/js/l10n') directly.

There one more failing test in https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=45421be4c921 which I'll fix shortly.
Attachment #8559178 - Flags: review?(gandalf)
(In reply to Staś Małolepszy :stas from comment #6)
> (In reply to Zibi Braniecki [:gandalf] from comment #5)
> > It should not be listed in languages.json. It should be enabled by the flag
> > during build time imo.
> 
> I don't think I agree here. Adding build flags is hard. We don't want the
> flag to be turned on by default because we don't want the build time
> pseudolocale in production builds.

I disagree. We want this flag on for non-production builds and off for production builds by default.

> OTOH we do want the pseudolocale for all
> developer purposes. Adding it to languages.json is the closest thing we can
> do to imitate a real locale (like ar or fr today). 

Disagree. There's a substantial difference for me between a locale which is stored in form of resource files in gaia source repository and one that is generated dynamically at build time.

For the former, I want it in languages.json and meta tags. For the latter, I want it to be added when build time step is executed. That allows me to turn on/off the feature and get the proper languages.json/meta tags and resource files.

If you do one part of it programmatically at build time and another part of it, you end up with an inconsistent build.

> > 
> > > Suppose we go the way of not including it:  do you have any ideas about what
> > > to do with the app names in the manifests?
> > 
> > Sure. It should be injected into the manifest at build time using
> > pseudolocalized default locale name/desc.
> 
> Well yeah, I guess I was asking about the how. Is there a way to include the
> pseudolocale code in multilocale.js?  Or, do I need to duplicate that code
> between  build/l10n.js and multilocale.js?

Oh, that's an interesting question. I guess, in the spirit of "let's remove l10n.js from build" I'd love to see pseudolocale.js that we include in l10n.js be available as a standalone module that can be included into multilocale.js.
Hah, mid-air collisions FTW

(In reply to Staś Małolepszy :stas from comment #8)
> Comment on attachment 8559178 [details] [review]
> [PullReq] stasm:1101632-buildtime-qps to mozilla-b2g:master
> 
> Zibi, can you take a look at this approach?
> 
> I had to copy some of the pseudol10n code so that I could require it from
> multilocale.js and from webapp-optimize.js.  I put it in a separate file,
> build/l10n-pseudo.
> 
> Once bug 1129926 is fixed, we can remove this file and
> require('shared/js/l10n') directly.

I believe that we should go in the opposite direction and create ./build/l10n and have ./build/l10n/l10noptimizer.js and ./build/l10n/pseudolocale.js as modules there.
(In reply to Zibi Braniecki [:gandalf] from comment #9)

> > I don't think I agree here. Adding build flags is hard. We don't want the
> > flag to be turned on by default because we don't want the build time
> > pseudolocale in production builds.
> 
> I disagree. We want this flag on for non-production builds and off for
> production builds by default.

I'd like `make install-gaia` to build Gaia with this pseudolocale.  At the same time, I don't want it in production builds.  I' m not sure if I know how to achieve this via Makefile flags alone, knowing that we can't assume partners will include a new flag in their configs. Any ideas?
Keving, how would you like to see build-time pseudolocales to be populated?
Flags: needinfo?(kgrandon)
Are we going to get rid of runtime pseudolocales then? This might be worth a survey on dev-gaia, but I really don't like the idea of having both runtime and buildtime pseudolocales. I think we really only need a single solution here.

We may need to do some work to make a single solution friendly to all workflows. Maybe it entails doing something with our developer tools/webIDE. I realize I'm not answering the question directly - I'd like to understand the longer term vision here as I don't think two approaches is a good idea.
Flags: needinfo?(kgrandon)
(In reply to Kevin Grandon :kgrandon from comment #13)
> Are we going to get rid of runtime pseudolocales then? This might be worth a
> survey on dev-gaia, but I really don't like the idea of having both runtime
> and buildtime pseudolocales. I think we really only need a single solution
> here.

I would like to have both. They serve different purpose from my perspective.

Runtime can be run against third-party apps and turned on in production to verify if UI is localizable.
Build time is for our apps only, but represents better the non-default locale performance and code flow.
So if we can think of a solution that will work for third party apps, as well as run through the correct locale code paths we would be happy? I need to think about this a bit, but I am sure we can come up with a solution that accomplishes that.

For example, third party developers are likely using webIDE. Can we have a button or share some code with WebIDE that can push a pseudolocale to the device? I am sure there are other options, but I haven't been thinking of these. I'm still not convinced that having both runtime and buildtime pseudolocales is a good idea.

Should we ask dev-gaia?
Similar to Zibi, I think both runtime and buildtime pseudolanguages are beneficial, both in their own ways.

Runtime pseudolanguages help our QA efforts on production builds.  Testers can switch to a runtime pseudolanguage and test for localizability bugs without knowing any foreign language.  This is particularly useful for RTL testing.

Buildtime pseudolanguages are useful for Gaia developers who build their own Gaia and/or apps.  They're meant to replace ar, fr and zh-TW which are now committed in the repo but awfully outdated.  These pseudolocales need to be processed on buildtime into JSON files just like a regular language would to imitate the same runtime overhead.  However, they should not be part of production builds b/c we dont' want to waste disk space on them.

The devtools.qps.enabled setting (Settings > Developer > Pseudo-localizations) controls the visibility of both the runtime and the buildtime pseudo-languages in the list of available languages.
If it's just for QA, it seems easy enough to provide builds, bundle locales with their builds, or test on engineering builds. Maybe you could also download a pseudolocale langpack?

I really don't think we should have two solutions, but if it's what you guys think is the best solution feel free to move forward. I think there's an opportunity to have one solution that solves both problems, and going solely from the conversation in this bug, it seems that we haven't thought about possibly better solutions. I need to spend some time thinking about it, and if I think of anything better I will let you guys know.
FWIW, I like the idea of a pseudolocale langpack.  The only advantage of runtime pseudolocales is that they are always up-to-date which can be important. OTOH, we could build pseudo langpacks on a daily basis and push updates as needed.

Let's give ourselves a little bit more time to think this through.  There's no urgency here but just to let you know I'd like to remove ar, fr and zh-TW this quarter if we can find a good solution to replace them.
I actually think that langpack is testing *yet* another route.

Langpack is not a runtime replacement because it cannot generate strings out of en-US strings at runtime.
Langpack is not a build time because it doesn't test our build system producing correct data structures and our runtime reading those structures properly.

Langpack is testing the langpack route. It's valuable because we can produce arbitrary langpacks that are always up to date and because they are packaged they are closer to build time pseudolocale.

Yet another way to think about it is:

 - runtime pseudolocale is for app developers to test if their app is localizable.
 - buildtime pseudolocale is for gaia system to test if it produces proper l10n resources and loads them properly
 - langpack pseudolocale is for gaia system to test if it produces proper langpacks and loads them properly
Talking to Zibi last night, it occurred to me that we could use the information stored in <meta name="availableLanguages"> to differentiate between buildtime and runtime pseudolanguages.  I.e. the localization context can treat any qps-* locales that are listed in the <meta> as regular built locales and doesn't apply any special runtime logic to them.

I implemented this approach in the pull request and the results are promising.  We can effectively now specify any of the known qps-* locales in LOCALES_FILE (shared/resources/languages.json) and they will get built into JSON files.

This means that there's no need to add a new pseudolanguage (Mixedcase English was my previous suggestion);  instead we can build Accented English and Mirrored English if we chose to do so.

OTOH, we still get the benefit of runtime pseudolocales in production builds which were configured with a custom LOCALES_FILE.

I see two tests still failing, but feel free to start the review at your convenience, Zibi!
(In reply to Staś Małolepszy :stas from comment #21)
> I see two tests still failing, but feel free to start the review at your
> convenience, Zibi!

https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=56eac9a3a578 should be good now.
Kevin, what's your take on comment 21?  You had some reservations about having two solutions in place;  with this new approach, runtime only kicks in when qps were not built on buildtime and the devtools.qps.enabled setting is true.
Flags: needinfo?(kgrandon)
Seems better to me, still not sure if I'm a fan of need to the special runtime logic though. Thanks for letting me know!
Flags: needinfo?(kgrandon)
Comment on attachment 8559178 [details] [review]
[PullReq] stasm:1101632-buildtime-qps to mozilla-b2g:master

Ricky, can you take a look at changes to the build/ directory?  They're limited to the l10n logic but one thing that stands out is that I removed the check for LOCALE_BASEDIR in multilocale.js so that the manifests are localized into pseudolanguages.  I think it's okay:  I'm removing the non-en-US languages from the default LOCALES_FILE in this pull request as well so there's no risk this code tries to fetch l10n files it doesn't know about.

(If you pass a different LOCALES_FILE with more languages you'll also need to pass a proper LOCALE_BASEDIR, but I think this already is the case anyways.)
Attachment #8559178 - Flags: review?(ricky060709)
Look good in build script part to me. r=me thanks!
Attachment #8559178 - Flags: review?(ricky060709) → review+
Thanks, Ricky.

Zibi, I pushed one more commit which syncs the pull request with changes I ported to the l20n.js repo.  There are harmless whitespace changes in build/l10n/l10n.js because I changed the way it's built now (without the surrounding IIFE).

It might be easier to view the diff without the latest commit:

  https://github.com/stasm/gaia/compare/mozilla-b2g:master...1101632-buildtime-qps%5E
Zibi, ping?
Flags: needinfo?(gandalf)
Comment on attachment 8559178 [details] [review]
[PullReq] stasm:1101632-buildtime-qps to mozilla-b2g:master

lgtm! Sorry for late review Stas
Attachment #8559178 - Flags: review?(gandalf) → review+
Blocks: fxos-pseudolocales
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: