1115773 - Fallback mechanism keeps loading last locale for each missing entity

Status: RESOLVED FIXED

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

Description

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

It seems that there's a bug in our code, that I noticed while working on langpacks.

When an entity is missing from en-US (defaultLocale), we load it. When the next entity is missing we seem to start loading it again.

I didn't debug it yet, but I see in logs that if en-US.json is empty, we load it many times.

We should fix it because with misuse of mozL10n.get (for testing if the entity is empty) we will load en-US Locale for each call. :/

Comment 1

[Staś Małolepszy :stas]

Without any own debugging yet I wonder if it's the mutation observer that you see in the logs?  When we localize the HTML, the mutation observer calls getEntity, which starts loading the fallback locale.  Could it be that the next mutation is processed before en-US.json loads?  The loading should be synchronous but I'm not sure if I remember how exactly the mutation microtasks are executed.

Comment 2

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

idk. I'll try to reproduce it today.

Comment 3

[Staś Małolepszy :stas]

I started debugging this earlier today and all I was seeing was telling me our sync XHR isn't sync.  After exploring many possible reasons (I suspected Gecko changes) I had a closer look at our io.loadJSON method:

https://github.com/mozilla-b2g/gaia/blob/2a39e2f4fb9785ce6dd5a914f9b844021d59a5f6/shared/js/l10n.js#L52

It's always async :)

I'll have a patch when I'm back tonight.  I noticed that we can't override the MIME type to JSON anymore in sync requests ("InvalidAccessError: A parameter or an operation is not supported by the underlying object") so we'll need to JSON.parse manually, I think.

Comment 4

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

Gosh, thanks! I was considering panicking. :)

Comment 5

[Staś Małolepszy :stas]

Attachment: Allow loadJSON to be async

Comment 6

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

Comment on attachment 8545548 [details] [review]
Allow loadJSON to be async

I think there's still a major benefit in most common scenario to use responseType json. I'd like to preserve that.

Can we just check if it's sync and skip responseType and JSON.parse in the callback?

Comment 7

[Staś Małolepszy :stas]

OK, I'll update the patch.  I was hoping we can get rid of the redundant code here though :(

Comment 8

[Staś Małolepszy :stas]

Patch updated.

https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=c21afc56e8ee

Comment 9

[Staś Małolepszy :stas]

Oops, it looks like I broke all l10n tests ^^.

Comment 10

[Staś Małolepszy :stas]

Aaaand it's fixed now.  It looks like the treeherder is having a hiccup though;  I don't have the URL to a new build yet.

Comment 11

[Staś Małolepszy :stas]

Zibi, this is now ready for your review.

https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=f856cc98486e

Comment 12

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

Comment on attachment 8545548 [details] [review]
Allow loadJSON to be async

sweet, thanks!

Comment 13

[Staś Małolepszy :stas]

https://github.com/mozilla-b2g/gaia/commit/1acb64c3dd32e285c5775119c4a5a1ff4dbbd5a3
https://github.com/l20n/l20n.js/commit/f0785817a49c7ba1fc9a59d6484e89b42b94ec24