Add setHTMLUnsafe and parseHTMLUnsafe methods

[josepharhar]

This adds the Element.prototype.setHTMLUnsafe and Document.parseHTMLUnsafe methods as described here: https://github.com/WICG/sanitizer-api/blob/main/explainer.md

• At least two implementers are interested (and none opposed):
  • Chrome
  • Firefox (Add setHTMLUnsafe and parseHTMLUnsafe methods #9538 (comment))
• Tests are written and can be reviewed and commented upon at:
  • Implement parseHTMLUnsafe and setHTMLUnsafe web-platform-tests/wpt#41704
• Implementation bugs are filed:
  • Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1478969
  • Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1851575
  • WebKit: https://bugs.webkit.org/show_bug.cgi?id=261143
• MDN issue is filed: setHTMLUnsafe and parseHTMLUnsafe methods mdn/mdn#459

(See WHATWG Working Mode: Changes for more details.)

---

/dom.html ( diff )
/dynamic-markup-insertion.html ( diff )
/index.html ( diff )

[josepharhar]

@otherdaniel @mikewest

[mikewest]

Looks reasonable. A few comments inline, though I'd suggest it would be useful to get an HTML editor's opinion about the right way to do this integration. @domenic might have time?

[domenic]

OK, so the main remaining issue to be discussed do we want the template retargeting behavior, where templateEl.setHTMLUnsafe(string) modifies templateEl.contents instead of templateEl directly?

I tend to say yes. Although it's a bit magical:

• There's really never a situation where you want to actually modify children of the template element itself.
• If we were different from innerHTML, that would probably be surprising.
• If we don't add this, it's pretty difficult to update the template element's contents from a string.

The last one could be fixed, by adding setHTMLUnsafe() to DocumentFragment. (Then, templateEl.contents.setHTMLUnsafe() would work.) That might be a good idea anyway; I feel like I've seen lots of threads asking for it? Edit: I found the thread I was thinking of and I remember why this is hard. You lose the potential context. Which was a big debate in the sanitizer API circles already. So, let's not couple that to this :).

To expand on "it's a bit magical", the downside is basically that templateEl.replaceChildren(foo) or templateEl.appendChild(foo) will change the template element's contents directly, so maybe templateEl.setHTMLUnsafe(foo) should follow that pattern instead of following templateEl.innerHTML = foo.

Hmm, related question: should this method also exist on ShadowRoot, like innerHTML does?

[mfreed7]

OK, so the main remaining issue to be discussed do we want the template retargeting behavior, where templateEl.setHTMLUnsafe(string) modifies templateEl.contents instead of templateEl directly?

I tend to say yes. Although it's a bit magical:

• There's really never a situation where you want to actually modify children of the template element itself.
• If we were different from innerHTML, that would probably be surprising.
• If we don't add this, it's pretty difficult to update the template element's contents from a string.

The last one could be fixed, by adding setHTMLUnsafe() to DocumentFragment. (Then, templateEl.contents.setHTMLUnsafe() would work.) That might be a good idea anyway; I feel like I've seen lots of threads asking for it? Edit: I found the thread I was thinking of and I remember why this is hard. You lose the potential context. Which was a big debate in the sanitizer API circles already. So, let's not couple that to this :).

To expand on "it's a bit magical", the downside is basically that templateEl.replaceChildren(foo) or templateEl.appendChild(foo) will change the template element's contents directly, so maybe templateEl.setHTMLUnsafe(foo) should follow that pattern instead of following templateEl.innerHTML = foo.

I tend to agree with you - I think element.setHTMLUnsafe(foo) should behave as closely as possible to element.innerHTML = foo, to avoid confusion. And as you point out, it's a convenience feature to avoid the footgun of setting <template>'s actual children. Setting template content has always been a bit magical, and I think this new method should follow that magic pattern.

Hmm, related question: should this method also exist on ShadowRoot, like innerHTML does?

Seems like it really should. @josepharhar seems like something to add?

[josepharhar]

Hmm, related question: should this method also exist on ShadowRoot, like innerHTML does?

Seems like it really should. @josepharhar seems like something to add?

I'd need to figure out how to make the "context" element in this algorithm also be a shadowroot:
https://html.spec.whatwg.org/multipage/parsing.html#html-fragment-parsing-algorithm

There is at least one spot that seems nontrivial to modify:

Create a start tag token whose name is the local name of context and whose attributes are the attributes of context.

[domenic]

I think the best way to do that is:

• Extract all of what is currently setHTMLUnsafe() into something like "unsafe set HTML steps" given contextNode, target, and string.
• Change Element's setHTMLUnsafe() to call "unsafe set HTML steps" given this, this and string
• Define ShadowRoot's setHTMLUnsafe() to call "unsafe set HTML steps" given this's shadow host, this, and string.

[josepharhar]

given this's shadow host

Ok cool I wasn't sure if this was how it should work, thanks! I applied your suggestions

[annevk]

Should the declarative shadow tree PR be made ready for merge first? Currently this PR suggests some shared code paths that are meant to not be shared. I.e., DOMParser and parseUnsafeHTML will deal with declarative shadow trees differently.

[josepharhar]

please file an MDN issue and update the OP with a link to it

Done