search

w3c / DOM-Parsing

DOM Parsing and Serialization
https://w3c.github.io/DOM-Parsing/
Other
27 stars 14 forks source link

fragment parsing algorithm appears to be wrong when invoked from innerHTML setter of template element #61

Open pshaughn opened 4 years ago

pshaughn commented 4 years ago

It's possible there's some other piece here that I'm missing, but it looks like https://w3c.github.io/DOM-Parsing/#dfn-fragment-parsing-algorithm tells us to make a document fragment on the node document of the context element, and then append nodes to it. This has the potential to enqueue upgrade reactions on those nodes if they match custom element definitions.

However, if we're setting the innerHTML of a template, we want to stay away from the node document of that template element, using the associated inert template document instead and not enqueuing upgrade reactions.

https://w3c.github.io/DOM-Parsing/#dom-innerhtml distinguishes the template as a special case, but only after it's already done calling the fragment parsing algorithm; the upgrade reactions have already been queued before we check if the context object is a template.

The second case of this WPT test checks when upgrades happen in this situation, and browsers are passing it, which implies the specification isn't matching browser behavior: https://github.com/web-platform-tests/wpt/blob/master/custom-elements/reactions/Document.html

bathos commented 2 years ago

It looks like there are two points where the spec is saying to upgrade in the innerHTML setter with a template element: even if the parsing case you desribed here were fixed, I think “try to upgrade” for any given CE produced during parsing would still get hit at step 7.2.2 of insert a node. Is that accurate?

Browsers appear to do what I’d have figured is the correct behavior in terms of intent (i.e., they do not upgrade during parse or during insertion) but it seems like the innerHTML setter steps are pretty far removed from this. There also may be a mistake at step 3 because it assigns a new value to context object, but that seems to be a defined term, not a regular local variable. I’d have guessed an algorithm can’t assign to such things but am not certain.

(Seems like this would be a bit safer & easier to reason about if the definition in DOMParsing had remained generic but the HTMLTemplateElement interface had defined its own innerHTML attribute with its own steps, but I’m guessing it’s too late for that now.)

pshaughn commented 2 years ago

I haven't looked back at the relevant specs more recently to see what they say now, but https://github.com/whatwg/dom/issues/833 had to do with the question you're asking.