Make custom element definition trigger in-document upgrades #1204
Conversation
|
LGTM. |
domenic
force-pushed
the
fix-upgrades-on-define
branch
from
daa1b9d
to
87b0aa6
Compare
|
This needs rebasing, most likely due to #1209. |
| <li><p>Let <var>upgrade candidates</var> be all elements in <var>document</var> whose namespace | ||
| is the <span>HTML namespace</span> and whose local name is <var>localName</var>, in tree | ||
| order. Additionally, if <var>extends</var> is non-null, only include elements that have an | ||
| attribute named <code data-x="attr-is">is</code> whose value is <var>name</var>.</p></li> |
There was a problem hiding this comment.
This surprised me a little bit since it means that a script that runs during parsing can remove the is="" attribute and it'll have an effect. Should we not store it on an internal slot?
There was a problem hiding this comment.
Hmm, what scenario are you specifically concerned about? Here is my guess:
<!DOCTYPE html>
<script>
customElements.define("x-a", class extends HTMLElement {
constructor() {
super();
document.querySelector("p").removeAttribute("is");
}
});
</script>
<p is="x-b"></p>
<x-a></x-a>
<script>
customElements.define("x-b", class extends HTMLParagraphElement {}, { extends: "p" })''
</script>In this scenario, as you say, the x-a constructor runs and removes the is="x-b" attribute from the <p>. Then, when we later run customElements.define("x-b", ...), the <p> is not upgraded.
That seems totally expected to me. I guess it is a little contradictory with the idea behind the current sentence:
After a custom element is created, changing the value of the is attribute does not change the element's behaviour.
But I don't think it's surprising, and it doesn't even contradict a strict reading of that sentence.
There was a problem hiding this comment.
That or simply <p is="x-b"></p><script>document.querySelector("p").removeAttribute("is")</script>. I think I misremembered that we planned on guarding against that.
There was a problem hiding this comment.
Ah, right, that's of course a simpler repro.
I think there was a general sentiment that "is only gets read once" and that changing it doesn't impact things. But I don't think it was ever formalized, and for edge cases like this I don't think we specifically wanted to preserve the originally-parsed is value.
I'll rebase.
This was part of the previous conclusion in WICG/webcomponents#419, but never got added back to the spec: - WICG/webcomponents@c4a829a removed all auto-upgrading - WICG/webcomponents@70b9d8d only added it back for when you insert an element into a document, not for when you define an element.
domenic
force-pushed
the
fix-upgrades-on-define
branch
from
87b0aa6
to
f7b7616
Compare
|
Rebased. |
domenic
added
the
topic: custom elements
This was part of the previous conclusion in
WICG/webcomponents#419, but never got added
back to the spec:
removed all auto-upgrading
only added it back for when you insert an element into a document, not
for when you define an element.
@dominiccooney, thanks for finding. Not sure if this is the best way to spec it; advice appreciated. As noted in the new HTML comment in the source, it would be equivalent in spec language to just "try to upgrade" all elements in the document.
BTW the reason for "this CustomElementRegistry's relevant global object's Document object" instead of "the current global object's Document object" is because in cases like
otherWindow.customElements.define.call(window.customElements, "x-foo", XFoo)the actual element definition will be added towindow.customElements("this CustomElementRegistry"), so it should bewindow.documentthat gets crawled for upgrade candidates, nototherWindow(= "the current global object").