With bug 992473 landed, we now have MO for Node l10n. But with current code, we initialize MO only after we loaded the resources, which does not cover nodes injected into DOM at runtime, before we have the resources. That's visible in bug 1030094. We should fix it by initializing MO earlier and collecting nodes injected before resources are available and then translating them as soon as possible.

Comment on attachment 8459245 [details] pull request It seems to be fairly trivial, unless I missed something. I'll run perf tests because initializing MO earlier might cost sth, but except of that it seems pretty straightforward and it also covers DOM operations during language switch... :) Stas, what do you think?

(In reply to Zibi Braniecki [:gandalf] from comment #0) > But with current code, we initialize MO only after we loaded the resources, > which does not cover nodes injected into DOM at runtime, before we have the > resources. I think I need a quick memory refresher here. If nodes are added at runtime before we have the resources, shouldn't they be localized by the onReady's translateDocument? I was also hoping we could remove this very translateDocument call in favor of a MO which is registered very early on. In the patch you left translateDocument and you start observing when document becomes interactive. Why not register it even earlier?

(In reply to Staś Małolepszy :stas from comment #3) > I think I need a quick memory refresher here. If nodes are added at runtime > before we have the resources, shouldn't they be localized by the onReady's > translateDocument? Yes In non-default locale, that's what happens. But in pre-translated case, we do not do the initial translation. > I was also hoping we could remove this very translateDocument call in favor > of a MO which is registered very early on. In the patch you left > translateDocument and you start observing when document becomes interactive. > Why not register it even earlier? That's my ultimate goal as well. I just believe that taking smaller steps is safer when we deal with such a racy code. This patch only introduces coverage for cases we do not cover at all right now (and thus, we use translateFragment there). Because l10n.js is deferred in most apps, we still need translateDocument. I'd like to soon experiment with initializing it without defer and relying on parser feeding MO (and in result, removing the call to translateDocument), but I'm not sure about performance implications. Overall, it seems complex enough (we will still need take into account scenarios in which l10n.js gets loaded too late for MO to localize nodes from parser) that I'd prefer not to cover it in this patch.

Among others, I expect that it may be the cause of bug 1037637.

Comment on attachment 8459245 [details] pull request No significant perf/mem difference on Settings. Tests are green

Comment on attachment 8459245 [details] pull request r=me, with the comments below. I have to admit that our startup logic is hard to read and understand. I actually started working on a similar patch which simplifies the initialization step greatly. I'll attach it shortly to get your feedback. From the patch: + var pendingEntities = null; Please call this pendingElements. + waitFor('interactive', function() { + nodeObserver = new MutationObserver(onMutations.bind(navigator.mozL10n)); + nodeObserver.observe(document, moConfig); + }); The way this block is placed, it will not be executed in the dataset.noComplete edge-case in neterror. + if (pendingEntities) { + for (var i in pendingEntities) { + translateElement.call(this, pendingEntities[i]); + } + pendingEntities = null; } Please use a more idiomatic way to iterate over the elements of the array, e.g. /* jshint boss:true */ for (var i = 0, element; element = pendingElements[i]; i++) { As we've already seen with Cost Control, some apps extend standard objects' prototypes with custom methods even if they shouldn't.

Pull request https://github.com/mozilla-b2g/gaia/pull/22038 TBPL: https://tbpl.mozilla.org/?rev=da6c1c6caa2e&tree=Gaia-Try Gandalf, here's a slightly modified patch. The second commit on my branch is virtually the same as yours. The first one gets rid of excessive setTimeouts. I'll test the perf and report back here.

Comment on attachment 8460265 [details] [diff] [review] Similar patch, with more clean-up Review of attachment 8460265 [details] [diff] [review]: ----------------------------------------------------------------- The patch has good perf numbers: app-visually-complete is 20ms later than on master for default locale and 60ms sooner for non-default ones. With one more change described below, the default-locale diff goes down to 10ms. I'll post full numbers in an hour, off to a meeting now. ::: shared/js/l10n.js @@ +1397,2 @@ > } > + initResources.call(navigator.mozL10n); Putting this in a setTimeout improves the performance of the default-locale case, bringing it on the par with master.

moz-app-visually-complete measurements for the SMS app (milliseconds): default locale non-default locale --------------------------------------------------------------------- master 873 1008 gandalf's patch 903 1029 stas' patch 920 1003 stas' patch with setTimeout(initResources) 910 966

(In reply to Staś Małolepszy :stas from comment #9) > Putting this in a setTimeout improves the performance of the default-locale > case, bringing it on the par with master. s/master/your patch/

No significant differences apart from a slight speedup in Settings. Some apps returned test failures (represented by a — in the results).

A slight speedup in Calendar. I'm surprised speedups in Email and FM are not significant.

Comment on attachment 8460265 [details] [diff] [review] Similar patch, with more clean-up Review of attachment 8460265 [details] [diff] [review]: ----------------------------------------------------------------- Let's land this. It simplified the code and seem to work well according to my tests. Please, monitor the datazilla carefully, especially for regressions in moz-app-visually-complete

Commit: https://github.com/mozilla-b2g/gaia/commit/a0f7ae47f8a2ac21ae817c84cf1d45335ce3d98d Merged: https://github.com/mozilla-b2g/gaia/commit/29793743960686ee8d5c4617a36172241c8ae0d0 L20n.js: https://github.com/l20n/l20n.js/commit/ac086ac8a5f7b51519f90f241e17756f59198be7