868677 - HTML bindings don't know how to react to content being injected into localized node

Status: RESOLVED FIXED

(L20n :: HTML Bindings, defect, P1)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

When a document is localized using localize() and then content is injected into it, we need a way to:

a) localize them
b) add entities to the pool retrieved by localize

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: POC

This is a POC of a solution.

There are two possible approaches to this:

1) We require explicit information about nodes being injected (similarly to how Gaia does it now in Settings app)
2) We use mutationObservers

In the POC I'm using the latter approach, but I don't know how reliable it will be. The problem with MO is that they're disconnected from nodeInjection so I cannot "show the panel" after it's translated. I can inject the nodes, transition to the panel, and hope that they'll be localized before the transition is over.

But if it'll work, it's much cheaper and more reliable then explicit calls, so I want to give it a try.

I also added first method to localizeHandler - getEntity - it gets the entity and adds it's id to idsOrTuples array used by localize.

The POC works.

Comment 2

[Staś Małolepszy :stas]

Comment on attachment 745466 [details] [diff] [review]
POC

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

(In reply to Zbigniew Braniecki [:gandalf] from comment #1)

> 1) We require explicit information about nodes being injected (similarly to
> how Gaia does it now in Settings app)

> 2) We use mutationObservers
> 
> In the POC I'm using the latter approach, but I don't know how reliable it
> will be. The problem with MO is that they're disconnected from nodeInjection
> so I cannot "show the panel" after it's translated. I can inject the nodes,
> transition to the panel, and hope that they'll be localized before the
> transition is over.

It sounds like the delay in case of locale fallbacks might cause flicker / old translations to show up?  I wonder if option #1 isn't more bullet-proof in this case.

> But if it'll work, it's much cheaper and more reliable then explicit calls,
> so I want to give it a try.

The big problem with MutationObserver is that it is not supported by IE which pretty much rules it out.  IE supports Mutations Events[1] which are deprecated and I wouldn't like to maintain a separate code path just for IE.  Again, it looks like it would be safer to go for option #1 above.

In general, it seems like this doesn't end up being more reliable, sadly (I'm actually sad).  Plus, there's more questions, e.g. should we remove an id from idsOrTuples if the last node using it is removed?

I'm not sure if HTML bindings should try too hard to be smart.  I'd prefer to keep things simple for now and make this use case possible via ctx.localize.  In fact, it looks like HTML bindings' own localizeNode could be used for this, if only we exposed it as a method of HTMLElement (element.localize would be nice).

[1] https://developer.mozilla.org/en-US/docs/DOM/Mutation_events


> I also added first method to localizeHandler - getEntity - it gets the
> entity and adds it's id to idsOrTuples array used by localize.

There are two problems with this change:

1. in the patch you only return { getEntity } when localize is called and the context isn't ready.  If it is ready, localize returns what getMany returns, which is currently {}

2. more importantly the async version of localize cannot return getEntity, which is sync.  The following code will break if ctx is not ready:

    ctx.freeze();
   var localizeHandler = ctx.localize(['foo'], callback);
   alert(localizeHandler.getEntity('bar'));

We could add a function that adds the id to idsOrTuples only, without trying to return a localized string.  Do we need it?

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

so, ultimately we want node injection to work automatically.

It means that since we fire a single localize() block for document.body, any node injected into body should be added to this localize block in order to make it retranslate together with other elements in the block when language is switched or when globals are updated.

It means that what we want here is:

1) Node is injected
2) We recognize that the node has l10n-id attribute
3) We add it to idsOrTuples
4) We register its globals into the localize callback
5) We translate the node

mutationObserver might work here, but may also cause flicker and it doesn't work in IE10.

In result we decided with Stas to go for an explicit call into API on node injection, that will become deprecated and removed once we have a reliable way to do this automatically.

What it means is that we need to do a few things:

1) Figure out the right API. It may be a call next to appendChild/innerHTML or it may be a wrapper in form of document.l10n.injectNode(parent, node);
2) Figure out a way to add the l10n-id of the node (and its children) to idsOrTuples
3) Figure out a way to register globals

It seems like it won't be trivial :/

Comment 4

[Staś Małolepszy :stas]

Attachment: WIP

I gave it a shot and here's how far I got.

(In reply to Zbigniew Braniecki [:gandalf] from comment #3)

> 1) Node is injected
> 2) We recognize that the node has l10n-id attribute

With the patch, you need to call document.l10n.extend(['foo', 'bar']) explicitly.

> 3) We add it to idsOrTuples

Done.

> 4) We register its globals into the localize callback

Done.

> 5) We translate the node

Not yet done.

This is WIP.  I'm not sure if Localized should be a separate class.  For now, it was easier;  the alternative was to pass more arguments to localize.bind.  Also, I'm hoping that by storing methods on Localized.prototype, we'll be a little bit more memory-efficient.

I'm definitely not sure what the API on document.l10n should look like.

Feedback?

Comment 5

[Staś Małolepszy :stas]

(In reply to Staś Małolepszy :stas (needinfo along with cc, please) from comment #4)

> > 5) We translate the node
> 
> Not yet done.

I'm not quite certain how this should look like.  I'm assuming we don't want to simply invoke the callback for the whole document, registered with bindings' ctx.localize in localizeNode.

Comment 6

[Staś Małolepszy :stas]

(In reply to Staś Małolepszy :stas (needinfo along with cc, please) from comment #5)
> (In reply to Staś Małolepszy :stas (needinfo along with cc, please) from
> comment #4)
> 
> > > 5) We translate the node
> > 
> > Not yet done.
> 
> I'm not quite certain how this should look like.  I'm assuming we don't want
> to simply invoke the callback for the whole document, registered with
> bindings' ctx.localize in localizeNode.

How about this:

1) var handler = ctx.localize() (the initial localization of document)
2) when the developer wants to add a node, they create it and call node.localize on it
3) HTMLElement.localize goes over all descendant nodes with data-l10n-id, similar to what retranslate() does
4) The collected ids are used with handler.extend to add them to idsOrTuples and bind globals
5) descendant nodes are localized using getEntity, synchronously, with no callback;  thanks to 4), handler's callback already knows about them
6) the developer attaches the localized node to the DOM

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Staś Małolepszy :stas (needinfo along with cc, please) from comment #6)
> (In reply to Staś Małolepszy :stas (needinfo along with cc, please) from

> How about this:
> 

I like this approach.

I took your code and updated it to use this approach - https://github.com/zbraniecki/l20n.js/tree/868677-nodeinj

Word of warning - it seems that this patch slows us down on Gaia, so let's not land it until we figure out the cause.

Comment 8

[Staś Małolepszy :stas]

Attachment: Proposed patch

Here's a patch I was working on today.

91ff356 Localized
1c219cf Pass reference to the context to Localized
6c47e64 Localize.prototype;  add relocalize
45e4daa Expose handler.extend
861b3fa collectIds
6a9e065 Simplify function names
ecfcabe Remove initializeDocumentContext
e0381f4 l10n (passed to localize's callback) now has the reason (fixes bug 868696)
603db11 Remove Localized
686352c Add bindLocalize so that localize doesn't have to take a 3rd arg
2e57336 Don't extend the localizationHandler on the initial localization of document.body
5a6879e Don't allow duplicate items in idsOrTuples

Comment 9

[Staś Małolepszy :stas]

Comment on attachment 746966 [details] [diff] [review]
Proposed patch

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

A few notes from the otp conversation with Gandalf and a few random questions:

::: bindings/l20n/html.js
@@ +10,3 @@
>    var ctx = L20n.getContext(document.location.host);
>  
> +  ctx.addEventListener('error', console.warn);

Is console.warn OK here?

@@ +10,5 @@
>    var ctx = L20n.getContext(document.location.host);
>  
> +  ctx.addEventListener('error', console.warn);
> +
> +  document.l10n = ctx;

I decided to assign ctx to the current document, instead of Document.prototype.  It's considered bad practice to extend Document.prototype.  Moreover, I don't think we'd want to use the same ctx for new documents created with document.implementation.createDocument.

@@ +92,5 @@
> +        // and the idsOrTuples have already been set in ctx.localize
> +        for (var i = 0; i < nodes.nodes.length; i++) {
> +          localizeNode(nodes.nodes[i], nodes.ids[i], l10n);
> +        }
> +        if (l10n.reason.locales) {

We should check for existance of l10n.reason here, too, in case localizeDocuemnt was invoked synchronously.  This will happen if ctx is ready after document.body is available:  ctx.localize will check _isReady, and will just fire the callback. Reasons are passed only by callbacks invoked by RetranslationManagers.

::: lib/l20n/context.js
@@ +256,5 @@
> +      var bound = {
> +        // stop: fn
> +        extend: function extend(ids) { 
> +          for (var i = 0; i < ids.length; i++) {
> +            if (idsOrTuples.indexOf(ids[i]) === -1) {

As Gandalf noted, this will not work for tuples.  We'll need to find another solution.

Comment 10

[Staś Małolepszy :stas]

(In reply to Staś Małolepszy :stas (needinfo along with cc, please) from comment #9)

> As Gandalf noted, this will not work for tuples.  We'll need to find another
> solution.

Consider the following code (in initial HTML or in a node that you'll inject and call node.localize()):

    <p data-l10n-id="receivedFrom" data-l10n-args='{"fromUser": "Mark"}'></p>
    <p data-l10n-id="receivedFrom" data-l10n-args='{"fromUser": "Anne"}'></p>

and the string:

    <receivedFrom "A message from {{ $fromUser }}">

l20n/html's getNodes will return [['receivedFrom', {"fromUser": "Mark"}], ['receivedFrom', {"fromUser": "Anne"}]].

Context::getMany will return { receivedFrom: "A message from Anne" }.  Oops.

I recall a phone conversation in which we wanted to get rid of data-l10n-args in favor of context-wide data, and only allow per-entity args in explicit ctx.get and ctx.getEntity calls.  I'm not even sure anymore if we should allow tuples in idsOrTuples.

Gandalf, what do you think?

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 746966 [details] [diff] [review]
Proposed patch

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

without this patch: 1084ms (16)
with this patch: 1130ms (22)

That's 45ms cost, repetitive. We need to identify why and if it's needed.

::: bindings/l20n/html.js
@@ +137,5 @@
> +      args = idOrTuple[1];
> +    } else {
> +      id = idOrTuple;
> +    }
> +    var entity = l10n ? l10n.entities[id] : ctx.getEntity(id, args);

In case when someone will add a node and not add it to localize, we need to recognize that we don't have entity for this ID and maybe console.warn:

    if (!entity) {
      console.warn("Unknown entity: "+id);
      return;
    }

Comment 12

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 746966 [details] [diff] [review]
Proposed patch

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

::: lib/l20n/context.js
@@ +263,5 @@
> +          }
> +          if (!_isReady) {
> +            return;
> +          }
> +          var many = getMany.call(this, ids);

it seems that this will make us compile entities twice. Once here just to take globals, and second time while localizing the nodes that we're extending.

Comment 13

[Staś Małolepszy :stas]

What about comment 10?

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Staś Małolepszy :stas (needinfo along with cc, please) from comment #13)
> What about comment 10?

yeah, let's remove tuples (args) for now.

Comment 15

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: patch for a patch

That's how far I got today. The patch applies against your patch. It:

 - removes idsOrTuples in facvor of ids
 - moves Intl to loadManifest
 - changes the heuristic of concurrent event listeners on body loading and resource loading. Because on Unagi and Fx we always get body first, I made use of it and I feed a global variable with collected nodes, to be used by localize() once the resource is loaded.
 - reordered emitter and retranslate in setReady to avoid double firing of each retranslation block added by onReady callbacks

What should be done:

 - if for some reason resources are available earlier than document.body we should create empty localize([], cb); and then fire document.body.localize on it (to extend the localize block and translate nodes)
 - extend seems to compile entities only to get their globals, and then the same entities will be compiled again to translate nodes
 - consider replacing HTMLElement.localize with document.l10n.localizeNode or sth like that. HTMLElement.localize costs us ~30ms startup time.

Feel free to take any part of my patch that you'll find useful and land it at will.

Comment 16

[Staś Małolepszy :stas]

Comment on attachment 747300 [details] [diff] [review]
patch for a patch

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

::: bindings/l20n/html.js
@@ +53,5 @@
> +      fireLocalizedEvent();
> +    }
> +  }
> +
> +  function initializeContext() {

Why put this back into a separate function?

ctx.addEventListener can by called before freeze, same for document.l10n = ctx;.  Then, we're only left with ctx.freeze.

@@ +80,4 @@
>          var langList = Intl.prioritizeLocales(manifest.languages);
>          ctx.registerLocales.apply(ctx, langList);
>          manifest.resources.forEach(function(uri) {
> +          ctx.linkResource(uri.replace.bind(uri, re));

This looks like a regression.  See bug 868723.

Comment 17

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Staś Małolepszy :stas (needinfo along with cc, please) from comment #16)
> Comment on attachment 747300 [details] [diff] [review]

> Why put this back into a separate function?

For performance. freeze() fires async IO, while we wait for resources to load, we can do CPU intensive operations.
 
> ctx.addEventListener can by called before freeze, same for document.l10n =
> ctx;.  Then, we're only left with ctx.freeze.

Yeah, I moved it together with HTMLElement.prototype.localize to minimize the cost of loading those. Since we don't want to have HTMLElement.localize anyway, the other two may be cheap enough to move them back.
 
> @@ +80,4 @@
> >          var langList = Intl.prioritizeLocales(manifest.languages);
> >          ctx.registerLocales.apply(ctx, langList);
> >          manifest.resources.forEach(function(uri) {
> > +          ctx.linkResource(uri.replace.bind(uri, re));
> 
> This looks like a regression.  See bug 868723.

right

Comment 18

[Staś Małolepszy :stas]

Comment on attachment 747300 [details] [diff] [review]
patch for a patch

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

::: bindings/l20n/html.js
@@ +56,5 @@
> +
> +  function initializeContext() {
> +    ctx.freeze();
> +    ctx.addEventListener('ready', function() {
> +      localizeHandler = ctx.localize(nodes.ids, onLocalize);

I don't think we should wait for ready here. ctx.localize gives us that anyways.  I understand that you need nodes to be defined, but I like your idea of using [] here and document.body.localize() in onLocalize better.

::: lib/l20n/context.js
@@ +468,2 @@
>        _retr.retranslate(_available);
> +      _emitter.emit('ready');

Hmm, doesn't this mean that 'ready' will be fired after all callbacks invoked by _retr.retranslate finish?

I think the previous order was right.  Or, we could use SetTimeout in EventEmitter::emit.  I checked a few other implementations of event emitters and none of them does that, though.

Comment 19

[Staś Małolepszy :stas]

Comment on attachment 747300 [details] [diff] [review]
patch for a patch

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

::: bindings/l20n/html.js
@@ +119,5 @@
> +  function localizeNode(node, id, l10n) {
> +    var entity = l10n ? l10n.entities[id] : ctx.getEntity(id);
> +    if (!entity) {
> +      console.warn("Unknown entity: "+id);
> +      return;

Doesn't this bypass our entire fallback logic?

Comment 20

[Staś Małolepszy :stas]

Attachment: Patch v2

This patch addresses all the feedback from comment 15 onwards.  I'll attach a diff against your last commit in a second.

Comment 21

[Staś Małolepszy :stas]

Attachment: Diff against Gandalf's last patch

Here are the changes I made today.  (Attachment 747455 [details] [diff] is diffed against master, while this is diffed against Gandalf's last commit.)

Comment 22

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Staś Małolepszy :stas (needinfo along with cc, please) from comment #18)
> Comment on attachment 747300 [details] [diff] [review]
> patch for a patch
> 
> Review of attachment 747300 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: bindings/l20n/html.js
> @@ +56,5 @@
> > +
> > +  function initializeContext() {
> > +    ctx.freeze();
> > +    ctx.addEventListener('ready', function() {
> > +      localizeHandler = ctx.localize(nodes.ids, onLocalize);
> 
> I don't think we should wait for ready here. ctx.localize gives us that
> anyways.  I understand that you need nodes to be defined, but I like your
> idea of using [] here and document.body.localize() in onLocalize better.

The cost of that is that you call bindLocalize and bindGet logic twice at startup.

> ::: lib/l20n/context.js
> @@ +468,2 @@
> >        _retr.retranslate(_available);
> > +      _emitter.emit('ready');
> 
> Hmm, doesn't this mean that 'ready' will be fired after all callbacks
> invoked by _retr.retranslate finish?
> 
> I think the previous order was right.  Or, we could use SetTimeout in
> EventEmitter::emit.  I checked a few other implementations of event emitters
> and none of them does that, though.

Well, the previous order makes such code:

ctx.addEventListener('ready', function() {
  ctx.localize([...], cb);
});

fire twice - first time at initialization within "ready" cb, and second time in retranslate right after that.

Comment 23

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: patch against stas' patch

I took your patch v2 and made a few changes

 - I reorganized html.js once again. My reasoning is like this:
    - we don't care about l10n until we have nodes.
    - so the first thing we want to have is collect nodes
    - once we collected nodes, we want to fire localize ASAP
    - if localize is not available, set eventlistener

   This way we minimize the amount of code running (only one bindLocalize, only one getNodes) and it should be fairly clear.

 - I removed getEntity from translateNode. If someone has added a node to document.body and didn't register it via localizeNode, we should not touch that node.
 - I removed a few cases of iteration where you check for hasOwnProperty. I believe it's not needed for objects where you work on keys from the same object as you iterate over. It only matters when you iterate over one (unsafe) list of keys, and then work on those keys on another object.
 - I reorganized setReady again to emit 'ready' after retranslation.

With this code I get really good performance numbers, and I believe it to be ready to land.

Comment 24

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: patch v3

full patch against master

Comment 25

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: patch against stas' patch

agh, actually cannot just bind the localize callback if I want to reuse the nodes array. Need to make a local function.

Patch against stas' patch

Comment 26

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: patch v4

and the same thing as a full patch against master

Comment 27

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 747714 [details] [diff] [review]
patch v4

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

::: bindings/l20n/html.js
@@ +68,5 @@
> +
> +    if (ctx.isReady) {
> +      localizeBody();
> +    } else {
> +      ctx.addEventListener('ready', localizeBody);

eh, I like bind way more here... Maybe we could just create a global variable like:

var globalHandler = localizationReady.bind(this, nodes);

and inside localizationReady removeListener to the globalHandler?

Comment 28

[Staś Małolepszy :stas]

I'll review the patch on Monday, let's not rush this change. I'm still not sure about the order of emit('ready') and _retr.all. I'd like to test it a little bit more. 


I'm okay with your other changes to html.js, except I think we don't have ctx.isReady? Do we want to expose it, similar to document.readyState?

Comment 29

[Staś Małolepszy :stas]

Attachment: [Checked in] Patch part 1: Bound localize

The patch has grown bigger and I decided to split it into two self-contained parts:

 - part one, about Context::localize returning a bound object,
 - part two, about the changes to bindings logic.

We seem to be on the same page regarding part one, so here the patch with an r?.  I'll attach the other one in a second.

Comment 30

[Staś Małolepszy :stas]

Attachment: Patch part 2: document.l10n

Here's the second part, keeping Gandalf as the author.  This patch changes the logic in HTML bindings.

Comment 31

[Staś Małolepszy :stas]

Comment on attachment 748816 [details] [diff] [review]
Patch part 2:  document.l10n

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

::: bindings/l20n/html.js
@@ +68,2 @@
>  
> +    if (ctx.isReady) {

As I mentioned before, we don't expose this on ctx.

@@ +69,5 @@
> +    if (ctx.isReady) {
> +      localizeBody(nodes);
> +    } else {
> +      localizeBodyHandler = localizeBody.bind(this, nodes);
> +      ctx.addEventListener('ready', localizeBodyHandler);

I don't like the fact that we have this method, Context::localize, which we advertise as an async one, yet in our own code we prefer to listen to 'ready' events before calling it.

It doesn't feel right to me.  We should be the first consumers of our own API.  If Context::localize is too slow because it returns `bound` and calls `_retr.bindGet`, then let's not allow for it to be called when the context isn't yet ready.

::: lib/l20n/context.js
@@ +470,2 @@
>        _retr.all(_available);
> +      _emitter.emit('ready');

As I said on IRC, this feels to me as a  very serious change that we shouldn't be making.  This essentially means that 'ready' is fired only after *all* callbacks from all `localize`s finish.

Comment 32

[Staś Małolepszy :stas]

(In reply to Staś Małolepszy :stas (needinfo along with cc, please) from comment #31)

> ::: lib/l20n/context.js
> @@ +470,2 @@
> >        _retr.all(_available);
> > +      _emitter.emit('ready');
> 
> As I said on IRC, this feels to me as a  very serious change that we
> shouldn't be making.  This essentially means that 'ready' is fired only
> after *all* callbacks from all `localize`s finish.

Also, with this setup, DocumentLocalized ends up being fired before 'ready', which I don't think is what we want.

Comment 33

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Staś Małolepszy :stas (needinfo along with cc, please) from comment #31)
 
> @@ +69,5 @@
> > +    if (ctx.isReady) {
> > +      localizeBody(nodes);
> > +    } else {
> > +      localizeBodyHandler = localizeBody.bind(this, nodes);
> > +      ctx.addEventListener('ready', localizeBodyHandler);
> 
> I don't like the fact that we have this method, Context::localize, which we
> advertise as an async one, yet in our own code we prefer to listen to
> 'ready' events before calling it.
>
> It doesn't feel right to me.  We should be the first consumers of our own
> API.  If Context::localize is too slow because it returns `bound` and calls
> `_retr.bindGet`, then let's not allow for it to be called when the context
> isn't yet ready.

The primary reason why we go that way is because we don't know the nodes we're localizing. That's a major difference - the canonical use of localize is to wrap a small piece of code that localizes small number of known nodes around some given logic.

In our API case - we need to localize the whole document.

Second thing - the primary target platform for this release is amazingly constrained and it makes literally every call in HTML API costly. Optimizing logic here to make the startup experience better is a fair trade imho.

> ::: lib/l20n/context.js
> @@ +470,2 @@
> >        _retr.all(_available);
> > +      _emitter.emit('ready');
> 
> As I said on IRC, this feels to me as a  very serious change that we
> shouldn't be making.  This essentially means that 'ready' is fired only
> after *all* callbacks from all `localize`s finish.

I don't understand what's the problem with this approach.

You have two clusters of code that will be executed on ready:
 - bunch of functions that react to "ready"
 - bunch of functions bound to react to "ready"

Why is it better to block latter by the former and not the other way around?

> Also, with this setup, DocumentLocalized ends up being fired before 'ready', which I don't think is what we want.

Since both are fired synchronously, they're really fired one after another in a single process. I still don't get what's wrong with that.

We could avoid this problem by polling the list of callbacks, fire ready, and then cycle through them and execute the ones that we collected before "ready" being fired, but I believe it to be an overkill, especially for 1.0.

Comment 34

[Staś Małolepszy :stas]

Comment on attachment 748814 [details] [diff] [review]
[Checked in] Patch part 1:  Bound localize

Landed in https://github.com/l20n/l20n.js/commit/abce3b3c0bd916e1e44cf3897aec0529eacb96a8

Comment 35

[Zibi Braniecki [:zbraniecki][:gandalf]]

Bug 868677 - DOM injection, part2: HTML bindings retranslation. r=stas :

https://github.com/zbraniecki/l20n.js/commit/a89efbf159e0aed0709a6691424583accf6a21d4

I carried r=stas with Context.isReady from IRC