Html: custom-elements: A step to add "is" content attribute in "new"

Created on 25 Jan 2018  路  22Comments  路  Source: whatwg/html

4.13.1.2 Creating a customized built-in element says:

const plasticButton2 = new PlasticButton();
console.log(plasticButton2.localName);          // will output "button"
console.log(plasticButton2.getAttribute("is")); // will output "plastic-button"

However, I couldn't find a step to add is attribute in new case though I found it in document.createElement() case

custom elements

All 22 comments

Hmm yeah, in particular https://html.spec.whatwg.org/multipage/dom.html#htmlconstructor doesn't mention the is attribute. I thought that all element allocation went through https://dom.spec.whatwg.org/#concept-create-element but I guess that's not the case?

@annevk unfortunately directly calling the constructor does not go through that. Rather, concept-element-create calls the constructor, at least for custom elements.

Because clone algorithm refers to is attribute value, cloneNode() for customized built-in elements created by constructors produce non-customized elements according to the current specifications.

@domenic how does that work then if you have your own constructor for a builtin?

@annevk I'm not sure what your question is. How does the existing spec work? It works pretty well; just follow the algorithms. (In particular the super-call to [HTMLConstructor] is what creates the element.) But, it fails to add the is="" attribute, as @tkent-google noted.

I started on this yesterday, but got a bit stuck as there's a clash with the requirement in create-an-element which disallows new attributes being added by the constructor. I'll try again today.

Cloning using the is attribute instead of the is value also seems like a bug.

I've wrote up most of a patch for this, but it's getting fairly complicated. I'd like implementer opinions from @tkent-google and anyone else working customized built-ins; I've previously seen @cbrewster and @bzbarsky from Mozilla pop in.

Here are some important background constraints. Assume we have a setup like customElements.define("plastic-button", class PlasticButton extends HTMLButtonElement { ... }, { extends: "button" }).

  • When doing new PlasticButton(), our only opportunity to run implementation code is during the super() call to HTMLButtonElement's constructor, i.e. during the [HTMLConstructor] algorithm.
  • Everything but new PlasticButton() goes through a central "create an element" algorithm. (For example, the parser or createElement().) In this algorithm, if the custom element is registered, we eventually call into the custom element custom constructor, and thus [HTMLConstructor].

My in-progress patch ends up looking like the following:

  • [HTMLConstructor], when it notices it's dealing with a registered customized built-in element, adds the is="" attribute (and sets the "is value").
  • "create an element", when passed an _is_ argument (e.g. via document.createElement(..., { is }) or the parser finding an is="" attribute in the token stream) need to add a check: did constructing the element cause the is="" attribute to be added? If so, great; the [HTMLConstructor] must have done that for us, so don't add it. If not, add it during this algorithm, and set the "is value" as well; this allows the element to get upgraded later if an appropriate definition is registered.
  • The parser omits is="" attributes from the list that it appends to the element due to parsing. By the time the element is created, via "create an element", the is="" attribute already exists, per the previous bullet. (This has impact on the ordering in the NamedNodeMap, making is="" always first, even if in source order it's not.)
  • Cloning looks at "is value", not is="" attribute; this is just a bugfix.

A simpler alternative would be the follows:

  • If you use new PlasticButton(), you're opting out of the is="" attribute reflecting the correct state. The internal "is value" is still correct. But in this world the is="" attribute is more of a tool for markup to trigger the correct element creation.
  • I guess in this world it would be more consistent to not set the attribute from createElement() either? So only the parser cares about is="", and only about reading it.
  • In this version, the spec mostly stays as it is today, except:

    • We probably remove setting the is="" attribute from createElement()

    • We fix cloning to look at "is value", not is="" attribute.

    • We fix the example quoted in the OP.

Basically the question is whether we want try hard to ensure that customized built-in elements, after they are created, have an is="" attribute that matches their is value. I think it might be OK to say "no"; we're already fairly clear that is="" is not a dynamic thing, e.g. modifying it does not have any effect on the element. (Except for, due to a current spec bug, what happens when you clone it.)

What do folks think?

My gut instinct is that changing parser behavior is pretty fishy....

The current custom element situation as Mozilla is ... temporarily in flux. @smag---- do you know who's on top of them right now?

Er, @smaug---- see above.

I like the simpler alternative much better. We should perhaps complement it by adding readonly attribute DOMString internalIs;, although writing it out I realize that's a terrible name due to the l and uppercase I.

Should we support serialization&parsing round-trip? e.g.

element.appendChild(new PlasticButton());
element.innerHTML = element.innerHTML;
element.lastChild instanceof PlasticButton; => should be true?

I think we should. With the simpler alternative that would mean special casing the serializer. That still seems better than further special casing the parser and might be reasonable anyway to override later-set is attributes on such elements?

Definitely the simpler thing. Having inconsistent attribute handling in parser feels rather bad.
That does mean inconsistency in serializer, but I guess I'd rather take that.
Something like, when serializing, and there isn't is attribute set explicitly, but the element is customized built-in, inject is attribute.

Fascinating, I hadn't anticipated people would want to change the serializer. OK, great, I'll work on this path.

@annevk I thought about adding an is getter, but I couldn't think of a use case, so I'll leave it out for now?

@domenic fine with me.

I've done two PRs that people could take a look at:

Copying @whatwg/components to ensure nobody gets surprised by this change.

Is the only remaining thing here filing a bug against Firefox and Safari? (I'm assuming @tkent-google took care of Chrome.)

@dstorey does Edge need a bug for this or are you earlier in your implementation stage?

We're not at that point yet, so as long as there are tests covering this it should be fine without a bug.

Yes, I take care of Google Chrome.

@domenic could you please link the Firefox and Safari bugs?

Safari does not implement customized built-ins. Gecko's bug is https://bugzilla.mozilla.org/show_bug.cgi?id=1440382.

Was this page helpful?
0 / 5 - 0 ratings