882591 - l10n.js should expose an asynchronous `localize()' method

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Fabien Cazenave [:kaze]]

TL;DR:
 • waiting for the `localized' event before startup is a waste of time;
 • when the language is changed, some apps aren’t fully translated;
 • both issues can be fixed by using an async `mozL10n.localize()' method.

----

Currently, all scripts that require l10n resources must wait for the `localized' event to be sent before being initialized.  This is overkill a few cases where we don’t need any mozL10n.get() at startup.

Besides, most of the time we use mozL10n.get() to change the content of an HTML element dynamically; but if the language is changed, the element won’t be re-translated unless its l10n-id and l10n-args attributes have been updated along with the content.

To fix this we experimented an asynchronous `localize()' function in the Settings app [1]. Instead of writing:

  mozL10n.ready(function initMyWholeApp() {
    […]
    element.textContent = mozL10n.get('message', params);
    element.dataset.l10nId = 'message';
    element.dataset.l10nArgs = JSON.stringify(params);
    […]
  });

we could use:

  mozL10n.localize(element, 'message', params);

… without having to care about the first mozL10n initialization, since the textContent will only be updated once mozL10n is ready.


Proposal: expose this `localize()' method in mozL10n and make it the preferred way to change an element content dynamically. The code is already there, it’s just a matter of moving the `localize()' function from Settings to mozL10n and start using it in Gaia apps.

[1] https://github.com/mozilla-b2g/gaia/commit/a46954a3#diff-1

Comment 1

[Fabien Cazenave [:kaze]]

Attachment: link to pull request

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

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 764180 [details]
link to pull request

r+ except for the comment. Re-request for review if you think I should look at it one more time. Thanks!

Comment 3

[[e-mail], [e-mail]]

Comment on attachment 764180 [details]
link to pull request

Kaze,

The asynchronous `localize()' method works fine for me. I have a test in Clock app with WIP. Thanks for your magic l10n help.

Ref:
https://github.com/ian-liu/gaia/tree/localize/Bug882591_expose_an_asynchronous_localize

Comment 4

[Fabien Cazenave [:kaze]]

Comment on attachment 764180 [details]
link to pull request

I’ve just updated the PR significantly, and a new review from you would help. :)

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 764180 [details]
link to pull request

Wait, why do you need the |localizeWhenReady()| function and the queue? Every element in the queue should be in the DOM tree, so it will always be localized when ready.

I think I was wrong on the previous comment; To fit the use case, we could just keep your current |localize()| function as-is and remove the |localizeWhenReady()| function.

Comment 6

[Fabien Cazenave [:kaze]]

The current localize() method is synchronous: it will only work if mozL10n is ready. Thus, it doesn’t help much (I had a edge case in mind where it could be useful, but I might have to think about it twice).

The new localizeWhenReady() method is asynchronous: you can use it even if mozL10n is not ready yet, which means apps don’t have to wait for the `localized' event to start. When applied to a single HTML element, it behaves like the `localize()' method did in the previous patch.

The problem with the previous patch was (as you mentioned) that if mozL10n is ready, every HTML element was localized one by one in its own setTimeout() call, which would have caused multiple reflows. With the current patch, all elements to be localized are queued: there’s no additional delay but it ensures these elements are all translated with the same setTimeout() call — see this line:

        timeoutID = timeoutID || setTimeout(localizeElementQueue);

Comment 7

[Fabien Cazenave [:kaze]]

Attachment: link to pull request — proper fix

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

Here’s a much simpler approach, which should work just as well. Thank you Tim for your explanations, it took me some time but I got them. :-)

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 765336 [details]
link to pull request — proper fix

woot!

Comment 9

[Fabien Cazenave [:kaze]]

Merged on master:
https://github.com/mozilla-b2g/gaia/commit/f8efe1a25aeaa7fd5443ba2bf2a823f7dc266864

Comment 10

[Salvador de la Puente González [:salva]]

we need this patch to make work bug 883816. Asking leo?

Comment 11

[Leo]

Bug 883816 is dependent on this.So marking leo+

Comment 12

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted f8efe1a25aeaa7fd5443ba2bf2a823f7dc266864 to:
v1-train: 11263e711d147ad46998287e70981f5101add737

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

1.1hd: 11263e711d147ad46998287e70981f5101add737