1135199 - GAIA_DEFAULT_LOCALE creates a mixed UI with English + translations

Status: RESOLVED FIXED

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

Description

[Théo Chevalier]

On 2.2 branch of our community builds for Open C device (http://builds.firefoxos.mozfr.org), using GAIA_DEFAULT_LOCALE=fr was causing homescreen + Settings app to be displayed in English, even if the French locale is complete.

A new build without this build option fixed the issue. So we're guessing there's a regression on 2.2 with GAIA_DEFAULT_LOCALE.

Comment 1

[Théo Chevalier]

Note, master does not seem affected, only 2.2 branch so far.

Comment 2

[:gerard-majax]

master is affected according to bug 1147332 comment 14

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

The only thing I see unusual so far is that when I plug WebIDE to Settings app, the mozL10n.ctx.locales.fr.entries is short (41 entries). fr.json is normal.

Stas, do you know what might be causing it?

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Hoooah...

Ok, I got it.

So, I started digging into why Settings' locale.fr.entries is only 50 elements in size.

I identified that here: https://github.com/mozilla-b2g/gaia/blob/master/shared/js/l10n.js#L1229-L1231 - the loop goes only 49 times (even when it's empty), but if I replace it with:

    for (var i = 0; i < ast.length; i++) {
      this.entries[ast[i].$i] = createEntry(ast[i], this.entries);
    }

it properly builds everything.

The reason here is that for Settings, ast[50] is "" which is falsy so the loop stops*.

I confirmed it inside JSON, the piece looks like this: `{"$i":"byteUnit-TB","$v":"To"},"",{"$i":"fileSize","$v"`

So, I suspect it will be a regression from bug 1101632 or bug 1013831.


* it's really tricky to debug. I think this is a good reason not to use value assignment in conditions of for loops.

Comment 5

[Staś Małolepszy :stas]

Ugh, nice work debugging this.

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #4)
> * it's really tricky to debug. I think this is a good reason not to use
> value assignment in conditions of for loops.

True. I know I've been a proponent of assignment loops in the past, but that only works if we're certain that the array won't have falsy values in it.  I wonder why the 50th element of the AST ends up being an empty string.  Might this be a bug in the parser?  In any case, let's change our loops to something more robust, so that we don't have to waste time in the future to debug bugs like this.

Comment 6

[GitHub Autolander Bot]

Attachment: [gaia] zbraniecki:1135199-fix-missing-entities-causing-partial-translations > mozilla-b2g:master

Comment 7

[Staś Małolepszy :stas]

Comment on attachment 8583691 [details] [review]
[gaia] zbraniecki:1135199-fix-missing-entities-causing-partial-translations > mozilla-b2g:master

r=me with a comment left on github. I think we should try to be consistent here: either change the looping style in build's addAST too, or don't change it in runtime's version.

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

Commit: https://github.com/mozilla-b2g/gaia/commit/497a7a13bda885bb8b672bbd59b3c6f957dce762

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8583691 [details] [review]
[gaia] zbraniecki:1135199-fix-missing-entities-causing-partial-translations > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): a combination of changes
[User impact] if declined: using GAIA_DEFAULT_LOCALE different than en-US results in some missing entities
[Testing completed]: device, tests
[Risk to taking this patch] (and alternatives if risky): not known
[String changes made]: none

Comment 10

[Zibi Braniecki [:zbraniecki][:gandalf]]

L20n.js: https://github.com/l20n/l20n.js/commit/ab90b7ebb23e01633e8f797d2d2f0db0d9a12db2

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

2.2: https://github.com/mozilla-b2g/gaia/commit/7876e6f16042c2a72cc736480865e7fb705d5963