Closed Bug 1007558 Opened 9 years ago Closed 9 years ago

Move l10n.js out of keyboard app start-up path


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

Gonk (Firefox OS)
Not set


(Not tracked)



(Reporter: timdream, Assigned: eeejay)



(Keywords: access, Whiteboard: [b2ga11y p=1])


(1 file)

+++ This bug was initially created as a clone of Bug #995025 +++

The aria label are programmatically get with _() function. This create a dependency and force l10n.js to load before keyboard.js et al. 

We want to move that out of the start-up path. The correct way to do things would be set the data-l10n-ids (say, 'foo') of the elements, and l10n.js will fill the labels from l10n properties with id like |foo.aria-label|.

I have a WIP patch but I haven't make sure it really works.

:eeejay, since you land the patch just yesterday, would you be able to work on this follow-up?
Flags: needinfo?(eitan)
Sure, I'll take this on. Thanks.
Assignee: nobody → eitan
Flags: needinfo?(eitan)
Your approach of adding nodes with data-l10n-id does not work, since the document is already loaded, and l10n.js only traverses the doc on load. But this really shouldn't be an issue, since the these translated strings are needed only at render time, and l10n.get is not called until the last moment.
Attachment #8419648 - Flags: review?(timdream)
Comment on attachment 8419648 [details] [review]
Link to Github pull-request:

I know it was r+'d but could you also move the |getAriaLabel()| and related stuff out of keyboard.js into render.js? We really need to trim that file.

Also, your patch still synchronously trying to translate a label when constructing a keyboard node. I am thinking whether or not we have a way to do that out asynchronously, like calling l10n.translate() on a node

I'll r+ if you do the first part in this bug. I can try to handle the second part later. Sorry for having you to work on such ill-maintained code.
Attachment #8419648 - Flags: review?(timdream)
s/the first part in this bug/the first part of the comment above/
Comment on attachment 8419648 [details] [review]
Link to Github pull-request:

Did what you asked!
Attachment #8419648 - Flags: review?(timdream)
Comment on attachment 8419648 [details] [review]
Link to Github pull-request:

Looks good, but please rebase again, sorry about that.
Attachment #8419648 - Flags: review?(timdream) → review+
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.