Closed
Bug 882591
Opened 11 years ago
Closed 11 years ago
l10n.js should expose an asynchronous `localize()' method
Categories
(Firefox OS Graveyard :: Gaia, defect)
Tracking
(blocking-b2g:leo+, b2g18 fixed, b2g-v1.1hd fixed)
RESOLVED
FIXED
blocking-b2g | leo+ |
People
(Reporter: kaze, Assigned: kaze)
References
Details
Attachments
(1 file, 1 obsolete file)
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
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → kaze
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #764180 -
Flags: review?(timdream)
Attachment #764180 -
Flags: feedback?(iliu)
Comment 2•11 years ago
|
||
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!
Attachment #764180 -
Flags: review?(timdream) → review+
Comment 3•11 years ago
|
||
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
Attachment #764180 -
Flags: feedback?(iliu) → feedback+
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 764180 [details]
link to pull request
I’ve just updated the PR significantly, and a new review from you would help. :)
Attachment #764180 -
Flags: review+ → review?(timdream)
Comment 5•11 years ago
|
||
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.
Attachment #764180 -
Flags: review?(timdream)
Assignee | ||
Comment 6•11 years ago
|
||
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);
Assignee | ||
Comment 7•11 years ago
|
||
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. :-)
Attachment #764180 -
Attachment is obsolete: true
Attachment #765336 -
Flags: review?(timdream)
Comment 8•11 years ago
|
||
Comment on attachment 765336 [details]
link to pull request — proper fix
woot!
Attachment #765336 -
Flags: review?(timdream) → review+
Assignee | ||
Comment 9•11 years ago
|
||
Merged on master:
https://github.com/mozilla-b2g/gaia/commit/f8efe1a25aeaa7fd5443ba2bf2a823f7dc266864
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 10•11 years ago
|
||
we need this patch to make work bug 883816. Asking leo?
blocking-b2g: --- → leo?
Comment 12•11 years ago
|
||
Uplifted f8efe1a25aeaa7fd5443ba2bf2a823f7dc266864 to:
v1-train: 11263e711d147ad46998287e70981f5101add737
status-b2g18:
--- → fixed
Comment 13•11 years ago
|
||
1.1hd: 11263e711d147ad46998287e70981f5101add737
status-b2g-v1.1hd:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•