Bug 1576343 Comment 11 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

OK, I just spent far too much time on debugging this because this code is... super hairy. Anyway, it turns out the answer is much more mundane than something about XBL (not sure where that came from; there's only custom elements...) or mutation observers being broken.

In fact, the problem is almost apparent from a critical reading of the comments in DOMLocalization.cpp, if you ignore the code bits:

```cpp
  // 1. Collect all localizable elements.
  // 2. Check if the document has a prototype that may cache translated elements.
  if (proto) {
    // 2.1. Handle the case when we have proto.
    // 2.1.1. Move elements that are not in the proto to a separate array.
    // We populate the sequence in reverse order. Let's bring it
    // back to top->bottom one.
    // 2.1.2. If we're not loading from cache, push the elements that
    //        are in the prototype to be translated and cached.
    if (!proto->WasL10nCached() && !elements.IsEmpty()) {
       ...
    }
    // 2.1.3. If there are elements that are not in the prototype,
    //        localize them without attempting to cache and
    //        independently of if we're loading from cache.
    if (!nonProtoElements.IsEmpty()) {
    }

    // 2.1.4. Check if anything has to be translated.
    if (promises.IsEmpty()) {
      // 2.1.4.1. If not, resolve the mReady and complete
      //          initial translation.
    } else {
      // 2.1.4.2. If we have any TranslateElements promises,
      //          collect them with Promise::All and schedule
      //          the L10nReadyHandler.
    }
  } else {
    // 2.2. Handle the case when we don't have proto.

    // 2.2.1. Otherwise, translate all available elements,
    //        without attempting to cache them and schedule
    //        the L10nReadyHandler.
  }

  // 3. Connect the root to L10nMutations observer.
  ConnectRoot(*elem, rv);
```

The problem is step 2.1.4.1 vs. step 3. Specifically, marking the initial translation as complete in step 2.1.4.1 tells the proto content sink that this has happened, which [calls `MaybeDoneWalking`](https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/prototype/PrototypeDocumentContentSink.cpp#601) which calls `DoneWalking`, which [calls `mDocument->EndLoad()`](https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/prototype/PrototypeDocumentContentSink.cpp#658), which [calls `UnblockDOMContentLoaded`](https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/base/Document.cpp#7210) which fires a DOMContentLoaded event.

That wouldn't be a problem, except that this is all synchronous, and so is the JS that runs as a result of the DCL event.

But we haven't connected up the mutation observer yet, because that only happens when we call `ConnectRoot()` in step 3.

I dunno if this is easily fixable or no, because I don't know what (inadvertently or otherwise) depends on the current state. But the current state means all `document.l10n.setAttributes()` calls that are made from inside DCL event handlers (or elsewhere in the sync calltree from that event) are ignored when we have a proto cache copy of the document. I expect that's not only a problem for connection.js and the fxa sidebar, but probably elsewhere as well...
OK, I just spent far too much time on debugging this because this code is... super hairy. Anyway, it turns out the answer is much more mundane than something about XBL (not sure where that came from; there's only custom elements...) or mutation observers being broken.

In fact, the problem is almost apparent from a critical reading of the comments in DocumentL10n.cpp, if you ignore the code bits:

```cpp
  // 1. Collect all localizable elements.
  // 2. Check if the document has a prototype that may cache translated elements.
  if (proto) {
    // 2.1. Handle the case when we have proto.
    // 2.1.1. Move elements that are not in the proto to a separate array.
    // We populate the sequence in reverse order. Let's bring it
    // back to top->bottom one.
    // 2.1.2. If we're not loading from cache, push the elements that
    //        are in the prototype to be translated and cached.
    if (!proto->WasL10nCached() && !elements.IsEmpty()) {
       ...
    }
    // 2.1.3. If there are elements that are not in the prototype,
    //        localize them without attempting to cache and
    //        independently of if we're loading from cache.
    if (!nonProtoElements.IsEmpty()) {
    }

    // 2.1.4. Check if anything has to be translated.
    if (promises.IsEmpty()) {
      // 2.1.4.1. If not, resolve the mReady and complete
      //          initial translation.
    } else {
      // 2.1.4.2. If we have any TranslateElements promises,
      //          collect them with Promise::All and schedule
      //          the L10nReadyHandler.
    }
  } else {
    // 2.2. Handle the case when we don't have proto.

    // 2.2.1. Otherwise, translate all available elements,
    //        without attempting to cache them and schedule
    //        the L10nReadyHandler.
  }

  // 3. Connect the root to L10nMutations observer.
  ConnectRoot(*elem, rv);
```

The problem is step 2.1.4.1 vs. step 3. Specifically, marking the initial translation as complete in step 2.1.4.1 tells the proto content sink that this has happened, which [calls `MaybeDoneWalking`](https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/prototype/PrototypeDocumentContentSink.cpp#601) which calls `DoneWalking`, which [calls `mDocument->EndLoad()`](https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/prototype/PrototypeDocumentContentSink.cpp#658), which [calls `UnblockDOMContentLoaded`](https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/base/Document.cpp#7210) which fires a DOMContentLoaded event.

That wouldn't be a problem, except that this is all synchronous, and so is the JS that runs as a result of the DCL event.

But we haven't connected up the mutation observer yet, because that only happens when we call `ConnectRoot()` in step 3.

I dunno if this is easily fixable or no, because I don't know what (inadvertently or otherwise) depends on the current state. But the current state means all `document.l10n.setAttributes()` calls that are made from inside DCL event handlers (or elsewhere in the sync calltree from that event) are ignored when we have a proto cache copy of the document. I expect that's not only a problem for connection.js and the fxa sidebar, but probably elsewhere as well...

Back to Bug 1576343 Comment 11