1012702 - Make app names localizable into pseudolocales

Status: RESOLVED FIXED

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

Description

[Staś Małolepszy :stas]

Bug 900182 introduced the concept of pseudolocales.  Currently, pseudolocalizations don't affect app manifests (and thus, app names).

This will be needed for cases where tests want to check for a localized name of an app, like in bug 921939 which currently uses French.  It is thus blocking bug 1011519.

Comment 1

[Staś Małolepszy :stas]

WIP: https://github.com/mozilla-b2g/gaia/pull/22532

Comment 2

[Staś Małolepszy :stas]

Green Try: https://tbpl.mozilla.org/?rev=85f6bc7fcbfb&tree=Gaia-Try

Comment 3

[Staś Małolepszy :stas]

Attachment: Patch 1

Pull request: https://github.com/mozilla-b2g/gaia/pull/22532

I counted five places in 

 1. verticalhome's grid,
 2. rocketbar,
 3. overlay when swiping from the edge,
 4. caption under task cards,
 5. Settings > App Permissions.

In the patch, I took care of Verticalhome in gaia_grid, and other places in manifest_helper.  The patch depends and requires the patch from bug 1041565, which should land very shortly.

Kevin, can you take a quick look at this?  Do you know if there are other places that I should check?

Comment 4

[Kevin Grandon :kgrandon]

Comment on attachment 8469571 [details] [diff] [review]
Patch 1

Review of attachment 8469571 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, but I don't think I can f+ this without trying some alternative solutions first. My understanding is that this for testing only, correct? If so, I prefer to find a solution that does not involve changing client code. I think there are some alternatives that we should explore, and only consider changing client code as a very last step. 

Have we considered an approach of building the pseudolocales into the app manifests at build time? Then everything would continue to work, and work with the new pseudolocales, no?

Comment 5

[Staś Małolepszy :stas]

(In reply to Kevin Grandon :kgrandon from comment #4)

> Sorry, but I don't think I can f+ this without trying some alternative
> solutions first. My understanding is that this for testing only, correct?

No, this is so that app names are displayed pseudolocalized in Gaia, on runtime.

> Have we considered an approach of building the pseudolocales into the app
> manifests at build time? Then everything would continue to work, and work
> with the new pseudolocales, no?

Yes, but I think that dynamically generating pseudolanguages is a better approach:  it's consistent no matter what the actual build setup is.  Otherwise GAIA_INLINE_LOCALES, GAIA_CONCAT_LOCALES, GAIA_PRETRANSLATE and DEBUG would all require special treatment, increasing the complexity of the code.

I also want users to be able to toggle pseudolanguages on and off in production builds;  that's why we removed locales-obj/qps-*.json files (and never produced pseudo-properties on buildtime) which would only take valuable space on the device.

Building pseudolanguages into the manifest on buildtime also means that this owuld only work for Gaia apps.  Doing it on runtime allows even the third party apps to benefit from the pseudolocalization testing (if they use Gaia's l10n.js).

Extending the ManifestHelper to support pseudo languages seems like a good solution which is limited in scope.  Do you know if there are other apps except Verticalhome that don't use it?

Comment 6

[Kevin Grandon :kgrandon]

What about extending the capability of data-l10n-id to pull out data from the manifest/app? E.g., data-l10n-id="manifest.locales.name", or something that pulls it out of the manifest.webapp file or even app object. You can then intercept these requests and do the action for core or third party apps.

It seems very strange to heavily discourage the use of .get(), then turn around and start implementing it for pseudo locales.

Comment 7

[Kevin Grandon :kgrandon]

Oops, I see we are using .translate() instead of .get(), but it still seems like a hack?

Comment 8

[Staś Małolepszy :stas]

(In reply to Kevin Grandon :kgrandon from comment #6)
> What about extending the capability of data-l10n-id to pull out data from
> the manifest/app? E.g., data-l10n-id="manifest.locales.name", or something
> that pulls it out of the manifest.webapp file or even app object. You can
> then intercept these requests and do the action for core or third party apps.

How would you use this data-l10n-id?  On which element?  Manifest data is not associated with any given DOM element.  I think overloading data-l10n-id would qualify as a hack.

> It seems very strange to heavily discourage the use of .get(), then turn
> around and start implementing it for pseudo locales.

> Oops, I see we are using .translate() instead of .get(), but it still seems
> like a hack?

It's called 'translate', but it could also be called apply, use, modify etc.  It's a method of Pseudo objects which transforms the given value into a pseudotranslation.  It doesn't have a state like get() does (the current language; list of resources; readiness of mozL10n), which is why I think it's safe to use here.

Comment 9

[Staś Małolepszy :stas]

Hey Kevin, I'm back from vacation and would like to move forward with this bug. What do you think about comment 8?

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 8469571 [details] [diff] [review]
Patch 1

Review of attachment 8469571 [details] [diff] [review]:
-----------------------------------------------------------------

I still think that having specialized runtime code for this is not a good idea, and I don't see value in running these on PRODUCTION profiles. I am willing however to f+ this and help you get it landed as it's pretty minor, and I think that any other solution would be a *lot* of code. These paper cuts and special cases add up over time though, and eventually the whole codebase suffers.

Comment 11

[Staś Małolepszy :stas]

Thanks, Kevin.  I too try to avoid growing the codebase too much.  One added value of running pseudolocales in production builds is that we'll be able to make it easier for community members and even outsourced QA people to test locales they don't speak;  this is especially important for RTL languages.

I rebased my patch and I'm waiting for the Try results:

https://tbpl.mozilla.org/?rev=fbd062fe2592&tree=Gaia-Try

Do you know if there's someone else whom I should ask for review?  Is your f+ good for landing?

Comment 12

[Kevin Grandon :kgrandon]

Comment on attachment 8469571 [details] [diff] [review]
Patch 1

Review of attachment 8469571 [details] [diff] [review]:
-----------------------------------------------------------------

You can land with my R+.

Comment 13

[Staś Małolepszy :stas]

Thanks, Kevin!

Commit: https://github.com/mozilla-b2g/gaia/commit/f998162201ff577573f81a2d053fde165108f699
Master: https://github.com/mozilla-b2g/gaia/commit/8f3fca981ad795670e6caaa2721ec98ae5f2a3a5