Mutation observability #15

bathos · 2022-03-10T22:08:45Z

A MutationObserver activated in a blocking script allows for the observation of insertion driven by document parsing itself (apart from those within templates).

I would have expected that declarative shadows (being declarative) wouldn’t reify and expose an actual <template> element — it seems like it’s supposed to just be the backwards-compatible syntactic hook for initializing a shadow root, not a “literal” HTMLTemplateElement. However in the current Chromium implementation, there is an HTMLTemplateElement observable to JS (both its connection and its removal).

The removal step (but not the addition step) is also exposed to legacy synchronous DOMMutation events, which isn’t normally true during document parsing afaik. These actually block the parser and yield to JS before the rest of the input stream is consumed. It doesn’t get any less declarative than that :)

I’m guessing these behaviors are incorrect, but if not it may need to be clarified explicitly somewhere.

<!DOCTYPE html>

<script>
  addEventListener("DOMNodeRemoved", logLegacyEvent, true);
  addEventListener("DOMNodeRemovedFromDocument", logLegacyEvent, true);
  addEventListener("DOMSubtreeModified", logLegacyEvent, true);

  new MutationObserver(arr => arr.forEach(logMutation)).observe(document, {
    childList: true,
    subtree: true
  });

  function logLegacyEvent({ type, target, relatedNode }) {
    console.warn(type, { target, relatedNode });
  }

  function logMutation(record) {
    for (let child of record.addedNodes) {
      console.debug("%O added to %O", child, record.target);
    }

    for (let child of record.removedNodes) {
      console.debug("%O removed from %O", child, record.target);
    }
  }
</script>

<!-- declarative shadow -->

<div id="shadow-host">
  <template id="shadow-template" shadowroot="closed">
    shadow template text
  </template>
</div>

<!-- contrasting behavior: not synchronously observable -->

<div id="normal-div">
  <template id="normal-template">
    normal template text
  </template>
</div>

The text was updated successfully, but these errors were encountered:

mfreed7 · 2022-03-11T02:03:10Z

Thanks for the detailed issue description. This behavior is actually described in the explainer, and it is intentional. In particular:

One additional change is made to the specification for HTMLTemplateElement: the .content property will return null for the <template shadowroot="open|closed"> element created during parsing. This will prevent scripts and MutationObservers from gaining access to the internals of the shadow root while parsing is taking place. Since the <template> element is only present during parsing, and is removed in step 10 above, this simply prohibits access during parsing.

And this section talks a bit about the motivation for attaching on the closing tag rather than the open tag. That decision pretty much requires there to be an actual HTMLTemplateElement in the tree, since parsing can yield at any point and the results to that point need to be stored somewhere in the tree.

Do you feel something is “broken” here, or does the existing behavior just not feel as clean as it could be? (If the latter, I do agree, but I think we’re here for good reasons.)

bathos · 2022-03-11T04:50:20Z

I don’t think those sections directly address this(?) (the points of observation anyway, as opposed to the reified element)

It’s likely that “yield at any point” wasn’t intended to be a 100% precise, but because it’s directly relevant: “too many points” !== “any point”. This issue concerns novel forms of observable parse state and document parse yielding introduced with this feature. In particular I’d be pretty surprised if creating a new way to interrupt document parse using legacy DOM mutation events was actually intentional.

(I.E. even if it’s necessary to reify-insert-remove for other reasons (e.g. custom element reaction), not dispatching DOMNodeRemoved can still be specified explicitly. There is no obligation for new API surfaces to expand the range of what can be done with a deprecated API that only exists for backwards compat, and given how problematic those events already are, it’s worth specifying this).

I think @annevk or @rniwa would be able to speak about this more accurately than I can, but I suspect still more could be done to reduce novel parser state observability here. I may be mistaken about that, but I’m pretty sure that at minimum those legacy events should not be getting dispatched.

mfreed7 · 2022-04-11T18:06:33Z

(I.E. even if it’s necessary to reify-insert-remove for other reasons (e.g. custom element reaction), not dispatching DOMNodeRemoved can still be specified explicitly. There is no obligation for new API surfaces to expand the range of what can be done with a deprecated API that only exists for backwards compat, and given how problematic those events already are, it’s worth specifying this).

Thanks for your comment - I did miss the point you were making. I agree that it would like be better not to add new triggers for the deprecated synchronous mutation events, especially at parse time. How difficult that will be to either implement or spec - that's a good question. I have opened a bug against Chromium to at least explore. Thanks!

mfreed7 · 2022-04-11T19:08:18Z

Turns out it was a 2-line change, and I'm going to treat it like a bug in the Chromium implementation. Essentially, the parser shouldn't generally be firing mutation events, and the Chromium code was inadvertently doing so by not calling the right function (that avoids mutation events). I'll get that landed soon, and Chromium v102+ should contain the new behavior.

Thanks for reporting this! I'm going to close it.

bathos · 2022-04-20T07:21:48Z

That’s awesome!

I still suspect it is possible that this could be specified in such a way that <template shadowroot="..."> is purely syntactic w/ no intermediately observable template element, but even if I am correct to think this, it would undoubtedly be much more complex to specify it that way, so I realize there may be a trade-off being made where it was felt that reducing specification complexity was more valuable than minimizing the “exposed” complexity.

However the sync mutation observability was the main thing I was concerned about. Sorry the issue was fuzzy: I should have kept the description clearly focused on that since reading it back now I can see how this wasn’t very clear at all — I was folding two different issues together (a bug report, it turns out, and a more “meta” design concern). Thanks for making sense of it anyway, confirming that the “bug part” was a bug, and fixing it!

mfreed7 · 2022-05-03T16:22:59Z

I still suspect it is possible that this could be specified in such a way that <template shadowroot="..."> is purely syntactic w/ no intermediately observable template element, but even if I am correct to think this, it would undoubtedly be much more complex to specify it that way, so I realize there may be a trade-off being made where it was felt that reducing specification complexity was more valuable than minimizing the “exposed” complexity.

Yes, this was definitely a calculated compromise. It would have been "better" to build the shadow root on the opening <template> tag, and parse directly into that root. But that opened much larger risks with the parser, and those risks were the reason this feature was initially cancelled in 2017/2018.

However the sync mutation observability was the main thing I was concerned about. Sorry the issue was fuzzy: I should have kept the description clearly focused on that since reading it back now I can see how this wasn’t very clear at all — I was folding two different issues together (a bug report, it turns out, and a more “meta” design concern). Thanks for making sense of it anyway, confirming that the “bug part” was a bug, and fixing it!

No problem! Thanks for raising the concern.

bathos · 2022-05-10T17:02:54Z

@mfreed7 I hit an issue in the Chromium impl during further exploration in this area that would probably be of interest to you: scripts in declarative templates are not currently flagged as parser-inserted at the point when they’re evaluated. This effectively makes strict-dynamic bypassable (and perhaps has other consequences given this isn’t the only scenario where this flag determines behavior). The Chromium responder was uncertain if this arose due to a spec issue or an implementation issue and you’d probably know the answer.

It’d probably(?) be uncommon for the stars to align just right such that this behavior ends up leverageable as a “useful” CSP bypass, fortunately, but it seems pretty clear that it’s unsound behavior. Since it may just be an implementation error (I’m unsure too), I figured I’d communicate it on this tangentially connected thread initially rather than open a new issue; if it turns out it is spec level though, it’d likely merit one.

mfreed7 · 2022-05-11T23:21:28Z

Thanks for bringing it to my attention! I commented on the bug.

mfreed7 closed this as completed Apr 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mutation observability #15

Mutation observability #15

bathos commented Mar 10, 2022 •

edited

Loading

mfreed7 commented Mar 11, 2022

bathos commented Mar 11, 2022 •

edited

Loading

mfreed7 commented Apr 11, 2022

mfreed7 commented Apr 11, 2022

bathos commented Apr 20, 2022 •

edited

Loading

mfreed7 commented May 3, 2022

bathos commented May 10, 2022 •

edited

Loading

mfreed7 commented May 11, 2022

Mutation observability #15

Mutation observability #15

Comments

bathos commented Mar 10, 2022 • edited Loading

mfreed7 commented Mar 11, 2022

bathos commented Mar 11, 2022 • edited Loading

mfreed7 commented Apr 11, 2022

mfreed7 commented Apr 11, 2022

bathos commented Apr 20, 2022 • edited Loading

mfreed7 commented May 3, 2022

bathos commented May 10, 2022 • edited Loading

mfreed7 commented May 11, 2022

bathos commented Mar 10, 2022 •

edited

Loading

bathos commented Mar 11, 2022 •

edited

Loading

bathos commented Apr 20, 2022 •

edited

Loading

bathos commented May 10, 2022 •

edited

Loading