Exception: TypeError: meta.availableLanguages is not an object

RESOLVED FIXED in Firefox OS v2.2

Status

Firefox OS
Gaia::L10n
P1
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: m1, Assigned: gandalf)

Tracking

(Blocks: 1 bug)

unspecified
2.2 S5 (6feb)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)

Details

(URL)

Attachments

(3 attachments)

+++ This bug was initially created as a clone of Bug #1115796 +++

Bug 1115796 seems to have broken both our master and v2.2 builds with a:

Exception: TypeError: meta.availableLanguages is not an object
initResources@resource://gre/modules/commonjs/toolkit/loader.js -> file:///build/v2.2/build/gaia/build/utils-xpc.js -> file:///build/v2.2/build/gaia/build/l10n.js?d=1422011516614:76:1
navigator.mozL10n.bootstrap@resource://gre/modules/commonjs/toolkit/loader.js -> file:///build/v2.2/build/gaia/build/utils-xpc.js -> file:///build/v2.2/build/gaia/build/l10n.js?d=1422011516614:43:5
HTMLOptimizer.prototype.process@resource://gre/modules/commonjs/toolkit/loader.js -> file:///build/v2.2/build/gaia/build/webapp-optimize.js:71:3
WebappOptimize.prototype.processFile@resource://gre/modules/commonjs/toolkit/loader.js -> file:///build/v2.2/build/gaia/build/webapp-optimize.js:632:3
WebappOptimize.prototype.execute@resource://gre/modules/commonjs/toolkit/loader.js -> file:///build/v2.2/build/gaia/build/webapp-optimize.js:699:3
execute@resource://gre/modules/commonjs/toolkit/loader.js -> file:///build/v2.2/build/gaia/build/webapp-optimize.js:729:1
exports.execute@resource://gre/modules/commonjs/toolkit/loader.js -> file:///build/v2.2/build/gaia/build/post-app.js:24:3
buildApps/<@resource://gre/modules/commonjs/toolkit/loader.js -> file:///build/v2.2/build/gaia/build/app.js:100:7
buildApps@resource://gre/modules/commonjs/toolkit/loader.js -> file:///build/v2.2/build/gaia/build/app.js:56:3
exports.execute@resource://gre/modules/commonjs/toolkit/loader.js -> file:///build/v2.2/build/gaia/build/app.js:148:3
CommonjsRunner.prototype.run@/build/v2.2/build/gaia/build/xpcshell-commonjs.js:169:5
run@/build/v2.2/build/gaia/build/xpcshell-commonjs.js:184:3
@-e:1:1
make[1]: *** [app] Error 1
make[1]: Leaving directory `/build/v2.2/build/gaia'
make: *** [gaia/profile.tar.gz] Error 2
make: *** Waiting for unfinished jobs....

---

Where does meta.availableLanguages come from?
(Reporter)

Comment 1

3 years ago
Probably the wrong way to avoid this exception:

diff --git a/build/l10n.js b/build/l10n.js
index 982e181..fc46718 100644
--- a/build/l10n.js
+++ b/build/l10n.js
@@ -72,8 +72,10 @@
       document.documentElement.dataset.noCompleteBug = true;
     }
 
-    this.ctx.registerLocales(meta.defaultLanguage,
-                             Object.keys(meta.availableLanguages));
+    if (meta.availableLanguages) {
+      this.ctx.registerLocales(meta.defaultLanguage,
+                               Object.keys(meta.availableLanguages));
+    }
   }
I think a possibly better way to avoid this would be to create empty properties of the meta object here:

https://github.com/mozilla-b2g/gaia/blob/c322468a7fc9e12a59774d34147f9c1754f1bf30/shared/js/l10n.js#L1638

I seems to me that for some reason this line is not run and meta.availableLanguages stays undefined:

https://github.com/mozilla-b2g/gaia/blob/c322468a7fc9e12a59774d34147f9c1754f1bf30/shared/js/l10n.js#L1731
(Reporter)

Comment 3

3 years ago
Would it help if I attached the output from the full gaia build?  or something else?  Thanks!
Do you know which app is being processed when this error occurs?  WebappOptimize or HTMLOptimizer might have the reference to the HTML file in question.
(Reporter)

Comment 5

3 years ago
(In reply to Staś Małolepszy :stas from comment #4)
> Do you know which app is being processed when this error occurs? 

I'll check in a bit, but that's brings up a possible lead.  We have some outoftree/ [1] apps in our build.  Does bug 1115796 (or some other recent l10n change on master/v2.2) assume something new of apps?


[1] https://github.com/mozilla-b2g/gaia/blob/b07dc29eb/Makefile#L427
On buildtime, each app is run through build/multilocale.js which extracts the information about the languages the app is available in from LOCALES_FILE (typically shared/resources/languages.json) and puts it in a <meta name="availableLanguages" …> for fast sync access.

One potential troublemaker might be that in bug 1115807, we assumed every app would already have that <meta> element present.  In that very bug Zibi added the required <meta> elements to all HTML files in Gaia:

  https://github.com/mozilla-b2g/gaia/commit/8ef355889432359407039bf2e1dc69106979d7cd

It's possible that an outoftree app doesn't have it defined and the querySelector in 

  https://github.com/mozilla-b2g/gaia/commit/8ef355889432359407039bf2e1dc69106979d7cd#diff-e4109b329f82ce3c6b17eff9a10ed670R230

returns null.  Zibi, does this sound like a possible cause of the failure?
Flags: needinfo?(gandalf)
Created attachment 8553872 [details] [diff] [review]
Patch 1

Michael, can you test if the error persists with this patch?

It would be helpful to get the source of one of the outoftree apps.  Can you confirm that index.html does not contain <meta name="availableLanguages" …>?
Flags: needinfo?(mvines)
PR: https://github.com/mozilla-b2g/gaia/pull/27642
(Reporter)

Comment 9

3 years ago
(In reply to Staś Małolepszy :stas from comment #7)
>
> It would be helpful to get the source of one of the outoftree apps.

Sure, here's one - https://github.com/mozilla-b2g/firefoxos-loop-client

> Can you confirm that index.html does not contain <meta name="availableLanguages" …>?

Yes, there's nothing like this in these apps - https://github.com/mozilla-b2g/gaia/commit/00f8a78ad81
Flags: needinfo?(mvines)
(Reporter)

Comment 10

3 years ago
I still hit the same exception with attachment 8553872 [details] [diff] [review].  Looks like l10n.js is trying to process /build_stage/loop/app/call_screen/call.html [1] at the time (although we have several other outoftree apps as well).

[1] https://github.com/mozilla-b2g/firefoxos-loop-client/blob/1.1/app/call_screen/call.html
(Assignee)

Comment 11

3 years ago
The problem is deeper. Localization will *not* work for apps like https://github.com/mozilla-b2g/firefoxos-loop-client in 2.2 or master they still link to the manifest.

Yes, we can sugarcoat the error to stop breaking the build, but the outoftree apps still need to be updated.
It's as simple as replacing:

https://github.com/mozilla-b2g/firefoxos-loop-client/blob/a38d96add0e7d5f9efd677f80a27a258a056c5e4/app/index.html#L41

with

<meta name="defaultLanguage" content="en-US">
<meta name="availableLanguages" content="de, en-US, es, it">

(the locales are taken from https://github.com/mozilla-b2g/firefoxos-loop-client/tree/master/app/locales )

So, I would even advice against fixing this bug, because if we fix it we will have to wait for l10n QA tests to report that those apps don't work.

I would suggest we instead place a warning for apps that link l10n.js and manifest.webapp stating that missing meta.availableLanguages and defaultLocale will break localizability.
Flags: needinfo?(gandalf)
(In reply to Michael Vines [:m1] [:evilmachines] from comment #10)
> I still hit the same exception with attachment 8553872 [details] [diff] [review]
> [review].  Looks like l10n.js is trying to process
> /build_stage/loop/app/call_screen/call.html [1] at the time (although we
> have several other outoftree apps as well).

I linked firefoxos-loop-client/build in outoftree/firefoxos-loop-client and ran make profile.  I didn't see any error and I can confirmed that with my patch the resulting HTML as the required <meta> elements.

I'm not sure what might be causing your setup to fail :(
(Reporter)

Comment 13

3 years ago
(In reply to Zibi Braniecki [:gandalf] from comment #11)
>
> So, I would even advice against fixing this bug, because if we fix it we
> will have to wait for l10n QA tests to report that those apps don't work.
> 
> I would suggest we instead place a warning for apps that link l10n.js and
> manifest.webapp stating that missing meta.availableLanguages and
> defaultLocale will break localizability.

Not all outoftree apps need to be localized, some are commercial apps (like the loop app) but others are just test apps and having to deal with l10n in them is an unnecessary burden.

If we can't get a quick fix/workaround here for v2.2 then can we back out bug 1115796 from v2.2 today please, while we figure out the best path.  The "[Risk to taking this patch]" field at bug 1115796 comment 15 did promise no regressions ;)
(Assignee)

Comment 14

3 years ago
(In reply to Zibi Braniecki [:gandalf] from comment #11)
> The problem is deeper. Localization will *not* work for apps like
> https://github.com/mozilla-b2g/firefoxos-loop-client in 2.2 or master they
> still link to the manifest.
> 
> Yes, we can sugarcoat the error to stop breaking the build, but the
> outoftree apps still need to be updated.

Actually, I'm wrong.

If we fix the build system to generate the <meta> it will start working properly since at build time we will update languages list.

The reason for my comment was because today the localizability of those apps don't work. My patch in bug 1115796 just exposed the bug that was there before since landing of bug 1115807.

--

Bottom line is - I suggest we land Stas's patch (once it starts working for Michael), but we add a warning if manifest.webapp is linked and metas are not present.
(Assignee)

Comment 15

3 years ago
(In reply to Michael Vines [:m1] [:evilmachines] from comment #13)
> If we can't get a quick fix/workaround here for v2.2 then can we back out
> bug 1115796 from v2.2 today please, while we figure out the best path.  The
> "[Risk to taking this patch]" field at bug 1115796 comment 15 did promise no
> regressions ;)

Bug 1115796 did not introduce a regression. It exposed a bug in the outoftree app that has been there since bug 1115807 landed.

In result, backing out bug 1115796 will just hide the error and you will discover that apps like loop are not working in any other locale once you start QA.
(Assignee)

Comment 16

3 years ago
(In reply to Michael Vines [:m1] [:evilmachines] from comment #10)
> I still hit the same exception with attachment 8553872 [details] [diff] [review]
> [review].  Looks like l10n.js is trying to process
> /build_stage/loop/app/call_screen/call.html [1] at the time (although we
> have several other outoftree apps as well).
> 
> [1]
> https://github.com/mozilla-b2g/firefoxos-loop-client/blob/1.1/app/
> call_screen/call.html

Michael, can you provide me STR for building loop?

I have gaia repository, I can clone loop client repo, but I'm not sure how to link it and how you build gaia with that.
(Assignee)

Updated

3 years ago
Flags: needinfo?(mvines)
(Reporter)

Comment 17

3 years ago
(In reply to Zibi Braniecki [:gandalf] from comment #15)
> Bug 1115796 did not introduce a regression. It exposed a bug in the
> outoftree app that has been there since bug 1115807 landed.
> 
> In result, backing out bug 1115796 will just hide the error and you will
> discover that apps like loop are not working in any other locale once you
> start QA.

Sure, I'm not saying that we don't try to fix this properly.  The point is that the patch from bug 1115796 turned our v2.2 tree red.  Put another way, if bug 1115796 caused inbound or m-c/master to be red it would have been backed out immediately.  That's the situation we are in here.  I don't mind keeping out master branch red while we figure this out but not v2.2.
(Assignee)

Comment 18

3 years ago
(In reply to Michael Vines [:m1] [:evilmachines] from comment #17)
> Sure, I'm not saying that we don't try to fix this properly.  The point is
> that the patch from bug 1115796 turned our v2.2 tree red.  Put another way,
> if bug 1115796 caused inbound or m-c/master to be red it would have been
> backed out immediately.

True. But that's what allows us to discover bugs like this before they hit master. We can't do that for outoftree apps, so we need to rely on people discovering it before it hit them.

On the other hand, would you prefer to wait for QA round later in the cycle to discover that your apps don't work properly or have a build error that forces you to fix them?

Once again, backing out will get your a green build but a broken app because you don't test for locale change scenario.

>  That's the situation we are in here.  I don't mind
> keeping out master branch red while we figure this out but not v2.2.

The fix is the same for both. I asked for STR, can you help me here? Stas says that his patch fixed it for loop for him. You say it still does not fix it for you. If you can provide me STR, I can try to reproduce your problem. Unfortunately outoftree apps using non-frozen APIs are hard to track and fix.

Of course, we can also fix loop by properly using meta instead of manifest, but you say that there are other outoftree apps that may be broken, and from what I understand I can't get a list of them so I can't test them and fix them properly.

So my plan right now is:
 - get STR from you
 - update Stas's patch to build <meta/> for apps that use obsolete HTML API
 - add a build time warning for those apps to remind them that they need to update

This will fix it for master and 2.2.
(Reporter)

Comment 19

3 years ago
(In reply to Zibi Braniecki [:gandalf] from comment #16)
>
> Michael, can you provide me STR for building loop?

Try this: 

$ git clone https://github.com/mozilla-b2g/gaia.git
$ git clone https://github.com/mozilla-b2g/firefoxos-loop-client.git
$ export GAIA_OUTOFTREE_APP_SRCDIRS=$(readlink -f firefoxos-loop-client)
$ cd gaia
$ make
(Assignee)

Comment 20

3 years ago
Created attachment 8553982 [details] [diff] [review]
patch2.diff

m1: can you try this patch? It fixes the bug and reports a bold warning in the build log that encourages users to fix their code and explains how. :)
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
(Assignee)

Comment 21

3 years ago
Created attachment 8554005 [details] [review]
pull request

PR with the fixes.
Flags: needinfo?(mvines)
(Assignee)

Comment 22

3 years ago
Ok, so the PR makes us more careful and guards against using non-initialized attributes. It also uses multilocale.js to create missing ones and build/l10n.js to work properly even if they are not there.

I also filed a follow up bug 1125405 to update loop client to use the metas.
(Assignee)

Comment 23

3 years ago
Comment on attachment 8554005 [details] [review]
pull request

I need a rescue reviewer since Stas and Ricky are asleep+weekend.
Attachment #8554005 - Flags: review?(kgrandon)
Comment on attachment 8554005 [details] [review]
pull request

I am told that there is a deadline here, so going to go ahead and leave an R+. (You should probably still get a review from a build peer, even if it's a retroactive one). I also recommend adding a test, or filing a follow-up bug to add a test if there's a reason why we can't write one today.
Attachment #8554005 - Flags: review?(kgrandon) → review+
(Assignee)

Comment 25

3 years ago
Comment on attachment 8554005 [details] [review]
pull request

Retroactive r? on ricky and stas.

What this patch does is:

 - in build/l10n.js guard against missing metas. 
   - If defaultLanguage is missing, it will pass null and registerLocales will use ctx.defaultLocale.
   - If availableLanguages is missing, it will pass null and registerLocales will skip it.
 - in build/multilocale.js it:
   - adds missing <meta> tags
   - moves the early exist to before buildMetas and updates the check so that we don't add those tags for files that don't do l10n.js
   - warns user if their HTML file is using obsolete manifest.webapp.
Attachment #8554005 - Flags: review?(stas)
Attachment #8554005 - Flags: review?(ricky060709)
(Assignee)

Comment 26

3 years ago
Comment on attachment 8554005 [details] [review]
pull request

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 1115807
[User impact] if declined: apps that didn't update to bug 1115807 will fail at build time
[Testing completed]: app that was failing builds now
[Risk to taking this patch] (and alternatives if risky): not known
[String changes made]: none
Attachment #8554005 - Flags: approval-gaia-v2.2?(bbajaj)
(Reporter)

Comment 27

3 years ago
Comment on attachment 8554005 [details] [review]
pull request

This patch fixes all the things over here.  Thank you.
Attachment #8554005 - Flags: feedback+
(Assignee)

Comment 28

3 years ago
Commit: https://github.com/mozilla-b2g/gaia/commit/b82bfe8641fc6cce817dc5a357e782c4c55479c9
Merge: https://github.com/mozilla-b2g/gaia/commit/6b922e0837de128d7f395ccfcbf015721f88cb2d
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 29

3 years ago
L20n.js: https://github.com/l20n/l20n.js/commit/c9af6033322488d463454339d57f59ea21c22e56
Comment on attachment 8554005 [details] [review]
pull request

LGTM.
Attachment #8554005 - Flags: review?(ricky060709) → review+
Comment on attachment 8554005 [details] [review]
pull request

r=me a posteriori.  Thanks, Zibi.

I'll echo what Kevin said above:  let's file a follow-up and add a test.  I think we could use a new test-l10n-* app in https://github.com/mozilla-b2g/gaia/tree/master/dev_apps for this.
Attachment #8554005 - Flags: review?(stas) → review+

Updated

3 years ago
blocking-b2g: 2.2? → 2.2+

Updated

3 years ago
Attachment #8554005 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
(Assignee)

Comment 32

3 years ago
2.2: https://github.com/mozilla-b2g/gaia/commit/cc88f3c3ffaef71a15f34e16e5cc17f289683dcc
(Assignee)

Comment 33

3 years ago
As per bug 1125405 this was not a regression. Loop client should only be built on v2.0.

But it's still a good patch to guard against obsolete l10n API use for outoftree apps, so I'm glad we have it :)
Keywords: regression
status-b2g-v2.2: --- → fixed
status-b2g-master: --- → fixed
Target Milestone: --- → 2.2 S5 (6feb)

Updated

3 years ago
Blocks: 1142220

Updated

3 years ago
No longer blocks: 1142220
You need to log in before you can comment on or make changes to this bug.