1117002 - Optimize L10n AST contains duplicated entities

Status: RESOLVED FIXED

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

Description

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

It's a regression from bug 1018108.

While switching internal AST of resources from Object to Array, we missed that as a result of that, we combine ASTs of all document contexts in the webapp. When the app has more than one document linking to the same l10n resource, we will duplicate its entities in the result JSON.

The offending code is here: https://github.com/mozilla-b2g/gaia/blob/698e6e8a098cc060b26cd6f25171633c4c7e739d/build/webapp-optimize.js#L140-L142

Comment 1

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

Attachment: pull request

Simple fix.

Comment 2

[Staś Małolepszy :stas]

Comment on attachment 8543193 [details] [review]
pull request

Nice catch, r=me.

Comment 3

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

Commit: https://github.com/zbraniecki/gaia/commit/fee0d623346c34b7eaa860141158f2fc57913391
Merge: https://github.com/mozilla-b2g/gaia/commit/b41b9d7a185fed99784a7e2cae032df40224831e

Comment 4

[Kevin Grandon :kgrandon]

Reverted for breaking the linter (also present in the gaia-try run): https://github.com/mozilla-b2g/gaia/commit/e127d88898a55f236ad3c6fa13e1105ad48b0f16

Log from try run: http://ftp.mozilla.org/pub/mozilla.org/b2g/try-builds/gaiabld-2b6157ab647a/gaia-try-linux64_gecko/gaia-try_ubuntu64_vm-b2gdt_test-gaia-linter-bm68-tests1-linux64-build3853.txt.gz

As a side-comment, it seems like code like this should probably come with a unit test?

Comment 5

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

(In reply to Kevin Grandon :kgrandon from comment #4)
> Reverted for breaking the linter (also present in the gaia-try run):
> https://github.com/mozilla-b2g/gaia/commit/
> e127d88898a55f236ad3c6fa13e1105ad48b0f16

Agh! Thanks Kevin. Fixing.

> As a side-comment, it seems like code like this should probably come with a
> unit test?

The whole code to build ab-CD.json should. And we don't have that yet. Our current plan is to decouple l10n.js from webapp-optimize after 2.2 and introduce a small module that will create the optimized jsons instead. This will come with tests.

Comment 6

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

Attachment: pull request

Carrying over r+ from stas.

Comment 7

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

Commit: https://github.com/zbraniecki/gaia/commit/892967a977c8a274396ae7465937ecc9a3f2b3d3
Merge: https://github.com/mozilla-b2g/gaia/commit/c2bf20d23851d5fda9f8f0ef0267db5f49152376