1003191 - Clean up bootstrap mozL10n API use in the fl app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Staś Małolepszy :stas]

The fl app currently works around the non-deterministic mozL10n.ready behavior by first waiting for window.onload.  Bug 993188 fixed mozL10n.ready and bug 993189 introduced mozL10n.once, which is even better-suited for initialization tasks.

Comment 1

[Staś Małolepszy :stas]

Attachment: Patch (--ignore-all-whitespace)

https://github.com/mozilla-b2g/gaia/pull/18790

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8414505 [details] [diff] [review]
Patch (--ignore-all-whitespace)

Let's ask the app owner.

Comment 3

[David Flanagan [:djf]]

Comment on attachment 8414505 [details] [diff] [review]
Patch (--ignore-all-whitespace)

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

r- because I can't find anything in the docs or in shared/js/l10n.js that indicates that the once() callback is guaranteed to be invoked after the onload event has fired.  

If once() will only call its callback after onload, then please update the MDN docs to say that, and go ahead and change the r- to r+.  Otherwise, please leave the onload handler and just change ready to once.  

Because the FL app is only involved by activity handler it can be sensitive about the way it starts up, so I don't want to risk changing the semantics and having the message handlers firing before onload.

Comment 4

[Staś Małolepszy :stas]

(In reply to David Flanagan [:djf] from comment #3)

> r- because I can't find anything in the docs or in shared/js/l10n.js that
> indicates that the once() callback is guaranteed to be invoked after the
> onload event has fired.  

I investigated this question in bug 1000806.  As long as mozL10n.ready and .once are called in deferred scripts, the callback is guaranteed to have access to an interactive DOM.  This is the case here.  However, the onload event might still fire in the future.

> Because the FL app is only involved by activity handler it can be sensitive
> about the way it starts up, so I don't want to risk changing the semantics
> and having the message handlers firing before onload.

I'll err on the safe side of caution and I'll change the patch and leave the onload handler.

Just curious, is there a reason why the fl app waits for onload instead of DOMContentLoaded?  Would document.readyState == 'interactive' be enough?

Comment 5

[Staś Małolepszy :stas]

Attachment: Change mozL10n.ready to .once

Comment 6

[Staś Małolepszy :stas]

Builds:

https://tbpl.mozilla.org/?tree=Gaia-Try&rev=1d2a7a0500d8
https://travis-ci.org/mozilla-b2g/gaia/builds/26771792

Comment 7

[David Flanagan [:djf]]

Comment on attachment 8434135 [details] [diff] [review]
Change mozL10n.ready to .once

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

Looks good.

You're right that I should probably use something faster than the load event here, but if anyone ever cares about the performance of this app, we can worry about that then.

Comment 8

[Staś Małolepszy :stas]

(In reply to David Flanagan [:djf] from comment #7)
> Looks good.

Thanks for the review!

> You're right that I should probably use something faster than the load event
> here, but if anyone ever cares about the performance of this app, we can
> worry about that then.

Sounds good to me, I was just curious :)

Commit: https://github.com/mozilla-b2g/gaia/commit/5d5445ea22005ccf9ac090f52e0d486e305982ae
Merged: https://github.com/mozilla-b2g/gaia/commit/dfcd69ca8b68f15479f22bc4eabd1066f2b0cdf5