Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make custom element definition trigger in-document upgrades #1204

Merged
merged 1 commit into from
May 10, 2016
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion source
Original file line number Diff line number Diff line change
Expand Up @@ -67660,7 +67660,7 @@ console.log(plasticButton2.getAttribute("is")); // will output "plastic-button"<
<code>Window</code> object.</p>

<pre class="idl">interface <dfn>CustomElementsRegistry</dfn> {
void <span data-x="dom-CustomElementsRegistry-define">define</span>(DOMString name, Function constructor, optional ElementDefinitionOptions options);
[<span>CEReactions</span>] void <span data-x="dom-CustomElementsRegistry-define">define</span>(DOMString name, Function constructor, optional ElementDefinitionOptions options);
any <span data-x="dom-CustomElementsRegistry-get">get</span>(DOMString name);
Promise&lt;void&gt; <span data-x="dom-CustomElementsRegistry-whenDefined">whenDefined</span>(DOMString name);
};
Expand Down Expand Up @@ -67815,6 +67815,22 @@ dictionary <dfn>ElementDefinitionOptions</dfn> {

<li><p>Add <var>definition</var> to this <code>CustomElementsRegistry</code>.</p></li>

<li><p>Let <var>document</var> be this <code>CustomElementsRegistry</code>'s <span
data-x="concept-relevant-global">relevant global object</span>'s <span
data-x="concept-document-window"><code>Document</code> object</span>.</p></li>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

<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>

<li><p>For each element <var>element</var> in <var>upgrade candidates</var>, <span>enqueue a
custom element upgrade reaction</span> given <var>element</var> and
<var>definition</var>.</p></li>

<!-- It is equivalent to just try to upgrade all elements in the document, and let "try to
upgrade" bail out, but this seems a bit more explicit. -->

<li>
<p>If this <code>CustomElementsRegistry</code>'s <span>when-defined promise map</span>
contains an entry with key <var>name</var>:</p>
Expand Down