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

Add custom elements to HTML #1012

Merged
merged 3 commits into from Apr 13, 2016
Merged

Add custom elements to HTML #1012

merged 3 commits into from Apr 13, 2016

Conversation

@domenic
Copy link
Member

@domenic domenic commented Apr 7, 2016

This upstreams custom elements into the HTML Standard, instead of having
them be in a separate spec that monkeypatches HTML extensively. This
builds on previous work in whatwg/dom#204.

See https://rawgit.com/w3c/webcomponents/15a203c8393aef0df7223ab1d43406aa11a7e71e/spec/custom/index.html
for the source material, in particular the sections labeled "HTML+". All
changes while upstreaming were editorial in nature.


Review appreciated, especially in the form of pushing fixups to the branch.

@@ -108,6 +108,7 @@
<li><code>var</code></li>
<li><code>video</code></li>
<li><code>wbr</code></li>
<li><span>custom tags</span></li>
Copy link
Member

@annevk annevk Apr 8, 2016

The plan is to rename this in a follow up commit?

Copy link
Member Author

@domenic domenic Apr 8, 2016

Yeah, I guess, if we ever figure something out.

Copy link
Member

@annevk annevk Apr 8, 2016

Well I think we need to. "tags" is just super wrong.

@annevk
Copy link
Member

@annevk annevk commented Apr 8, 2016

I should have asked this earlier, where exactly does NewTarget come from? Is that worthy of a note or a cross-reference?

@domenic
Copy link
Member Author

@domenic domenic commented Apr 8, 2016

I should have asked this earlier, where exactly does NewTarget come from? Is that worthy of a note or a cross-reference?

I think it would be ideal to cross-reference, but ES does not have a definition for it; instead as far as I can tell it's just an ambient floating thing all ES algorithms get. I'll file an ES bug.

<li><p>If <var>name</var> is <code>blink</code>, <code>bgsound</code>, <code>isindex</code>,
<code>multicol</code>, <code>nextid</code>, or <code>spacer</code>, return
<code>HTMLUnknownElement</code>.</p></li>
<!-- These have to be explicitly listed because technically we define these elements in HTML,
Copy link
Member Author

@domenic domenic Apr 8, 2016

This helps with https://www.w3.org/Bugs/Public/show_bug.cgi?id=27877, but probably doesn't completely take care of it, as basefont might still be confusing.

@domenic
Copy link
Member Author

@domenic domenic commented Apr 8, 2016

I pushed fixes addressing the issues I noted on @annevk's commit. One interesting issue it raised is defining element interface for obsolete elements better. I will open a new issue on that.

@domenic
Copy link
Member Author

@domenic domenic commented Apr 8, 2016

Should "Core concepts" be named "Processing model"? Maybe not, since so much of the processing model is defined elsewhere...

@@ -9574,26 +9574,28 @@ interface <dfn>HTMLUnknownElement</dfn> : <span>HTMLElement</span> { };</pre>
<span>HTML namespace</span> is determined as follows:</p>

<ol>
<li><p>If <var>name</var> is <code>blink</code>, <code>bgsound</code>, <code>isindex</code>,
<li><p>If <var>name</var> is <code>bgsound</code>, <code>blink</code>, <code>isindex</code>,
<code>multicol</code>, <code>nextid</code>, or <code>spacer</code>, return
<code>HTMLUnknownElement</code>.</p></li>
<!-- These have to be explicitly listed because technically we define these elements in HTML,
albeit as obsolete. -->
Copy link
Member

@annevk annevk Apr 9, 2016

This comment is obsolete with this commit, no?

Copy link
Member Author

@domenic domenic Apr 9, 2016

Yeah good catch

@annevk
Copy link
Member

@annevk annevk commented Apr 9, 2016

Would you mind if I added ", then" to conditionals?

@annevk
Copy link
Member

@annevk annevk commented Apr 9, 2016

Although with all the don't squash that might get a little labor intensive...

@domenic
Copy link
Member Author

@domenic domenic commented Apr 9, 2016

You can add it, it's fine. Maybe don't add it to the sections touched in "don't squash" and I can do those myself when I rebase and rearrange commits.

@annevk
Copy link
Member

@annevk annevk commented Apr 13, 2016

I think it's all good now. I have fixed up the commits to add "then" and removed "don't squash" from the titles. Admittedly I have relied in part on my earlier review of the algorithms. Since I assume you just moved them around.

data-x="concept-custom-element-definition-name">name</span> <var>name</var>, then return that
entry's <span data-x="concept-custom-element-definition-constructor">constructor</span>.</p></li>

<li><p>Otherwise, return undefined.</p></li>
Copy link
Member

@zcorpan zcorpan Apr 13, 2016

I think it's more common to return null from a method. Why undefined?

Copy link
Member

@annevk annevk Apr 13, 2016

I noticed that. I suspect @domenic wants to mimic Map semantics.

Copy link
Member Author

@domenic domenic Apr 13, 2016

Yes, exactly, map-like get() methods need to return undefined.

@zcorpan
Copy link
Member

@zcorpan zcorpan commented Apr 13, 2016

There needs to be a normative statement saying that <html is> is invalid, e.g. "The is attribute must not be specified."

@zcorpan
Copy link
Member

@zcorpan zcorpan commented Apr 13, 2016

Do we want to allow any attribute on customized built-in elements? In particular those in the obsolete section? Note that embed allows any attribute except those that are obsolete (e.g. name).

@zcorpan
Copy link
Member

@zcorpan zcorpan commented Apr 13, 2016

+   <dt><var>window</var> . <code data-x="dom-window-customElements">customElements</code> . <code
+   data-x="dom-CustomElementsRegistry-define">define</code>(<var>name</var>,
+   <var>constructor</var>)</dt>

These typically have a subdfn attribute (which I think is for developers.whatwg.org?).

@zcorpan
Copy link
Member

@zcorpan zcorpan commented Apr 13, 2016

(Reached the end of the diff; no more comments.)

@domenic
Copy link
Member Author

@domenic domenic commented Apr 13, 2016

There needs to be a normative statement saying that <html is> is invalid, e.g. "The is attribute must not be specified."

Why?

Autonomous custom elements should also not allow the is attribute, right?

Good catch, thanks.

Shouldn't this use removeAttribute?

Either would work, I imagine. -1 is more explicit IMO?

Do we want to allow any attribute on customized built-in elements? In particular those in the obsolete section? Note that embed allows any attribute except those that are obsolete (e.g. name).

Hmm. We probably should? I have no real idea how to phrase this though, or which sections to edit. Suggestions?

These typically have a subdfn attribute (which I think is for developers.whatwg.org?).

Thanks, will add.

@zcorpan
Copy link
Member

@zcorpan zcorpan commented Apr 13, 2016

Either would work, I imagine. -1 is more explicit IMO?

-1 means it's still focusable if you click it. Also the code above uses removeAttribute and the prose talks about removing the attribute.

@zcorpan
Copy link
Member

@zcorpan zcorpan commented Apr 13, 2016

Why?

The content model is normative, but nothing says the attributes list is. The rest of the spec addresses each attribute individually in prose I believe. So right now it's a statement of fact that's not backed up by anything normative. :-)

@annevk
Copy link
Member

@annevk annevk commented Apr 13, 2016

Why can't is be specified on html anyway? It works, right?

@domenic
Copy link
Member Author

@domenic domenic commented Apr 13, 2016

Yeah, that was my question.

I agree with you now about removeAttribute; thanks for that.

@zcorpan
Copy link
Member

@zcorpan zcorpan commented Apr 13, 2016

Oh, I don't mind allowing is on html. Then remove , except the <code data-x="attr-is">is</code> attribute.

@zcorpan
Copy link
Member

@zcorpan zcorpan commented Apr 13, 2016

Hmm. We probably should? I have no real idea how to phrase this though, or which sections to edit. Suggestions?

How about...

diff --git a/source b/source
index 0967e3a..a627fa4 100644
--- a/source
+++ b/source
@@ -28002,8 +28002,8 @@ attribute, set the <span>browsing context name</span> of the element's <span>nes
   <p class="note">All attributes in <span>HTML documents</span> get lowercased automatically, so the
   restriction on uppercase letters doesn't affect such documents.</p>

-  <p class="note">The four exceptions are to exclude legacy attributes that have side-effects beyond
-  just sending parameters to the <span>plugin</span>.</p>
+  <p class="note">The four exceptions are to exclude <span>obsolete attributes</span> that have
+  side-effects beyond just sending parameters to the <span>plugin</span>.</p>

   <div w-nodev>

@@ -67011,9 +67011,13 @@ console.log(plasticButton2.getAttribute("is")); // will output "plastic-button"<

   <p>An <span>autonomous custom element</span> does not have any special meaning: it
   <span>represents</span> its children. A <span>customized built-in element</span> inherits the
-  semantics of the element that it extends. Both types of <span>custom element</span> may have any
-  non-namespaced attributes that are relevant to the element's functioning, as determined by the
-  element's author.</p>
+  semantics of the element that it extends.</p>
+
+  <p>Any namespace-less attribute, that are relevant to the element's functioning, as determined by
+  the element's author, may be specified on both types of <span data-x="custom element">custom
+  elements</span>, except those that are defined as being <span data-x="obsolete
+  attributes">obsolete</span> for that element, so long as its name is <span>XML-compatible</span>
+  and contains no <span>uppercase ASCII letters</span>.</p>

   <p>A <dfn data-export="">valid custom element name</dfn> is a sequence of characters
   <var>name</var> that meets all of the following requirements:</p>
@@ -111655,7 +111659,8 @@ if (s = prompt('What is your name?')) {

   <h3>Non-conforming features</h3>

-  <p>Elements in the following list are entirely obsolete, and must not be used by authors:</p>
+  <p>Elements in the following list are entirely <dfn data-x="obsolete elements">obsolete</dfn>, and
+  must not be used by authors:</p>

   <dl><!-- alphabetical by first element in the group, except CSS goes last -->

@@ -111736,8 +111741,8 @@ if (s = prompt('What is your name?')) {

   <hr>

-  <p>The following attributes are obsolete (though the elements are still part of the language), and
-  must not be used by authors:</p>
+  <p>The following attributes are <dfn data-x="obsolete attributes">obsolete</dfn> (though the
+  elements are still part of the language), and must not be used by authors:</p>

   <dl><!-- alphabetical by element then attribute of first item in group, except CSS goes last -->

Note: I copied the requirements from embed that the attribute name be lowercase and XML-compatible.

Additional thoughts:

  • The requirements on element names are similar but are defined differently. Editorial but is a bit inconsistent right now.
  • Why are any attributes allowed on customized built-in elements? Are they not as problematic for future extensions to the language as any attribute on standard elements?

@domenic
Copy link
Member Author

@domenic domenic commented Apr 13, 2016

Then remove , except the <code data-x="attr-is">is</code> attribute.

Huh, I didn't notice that. I think it must have been intended to go on autonomous built-in elements; I have no idea how it made it to <html>.

Why are any attributes allowed on customized built-in elements? Are they not as problematic for future extensions to the language as any attribute on standard elements?

That's a good point. Let's leave them invalid for now, and address this in a follow-up issue.

@domenic domenic force-pushed the customelements branch from 1804c62 to 3161de2 Apr 13, 2016
@domenic
Copy link
Member Author

@domenic domenic commented Apr 13, 2016

@zcorpan please take a look, made a fixup commit for easier reviewing.

@domenic domenic assigned zcorpan and unassigned domenic Apr 13, 2016
@zcorpan
Copy link
Member

@zcorpan zcorpan commented Apr 13, 2016

LGTM

domenic added 3 commits Apr 13, 2016
This upstreams custom elements into the HTML Standard, instead of having
them be in a separate spec that monkeypatches HTML extensively. This
builds on previous work in whatwg/dom#204.

See https://rawgit.com/w3c/webcomponents/15a203c8393aef0df7223ab1d43406aa11a7e71e/spec/custom/index.html
for the source material, in particular the sections labeled "HTML+".
Most changes while upstreaming were editorial in nature, with the
following exceptions:

- Changes were made to the "element interface" section to nail down
  underspecified edge cases, and fix https://www.w3.org/Bugs/Public/show_bug.cgi?id=27877.
- Changes were made to the authoring conformance criteria for attributes
  on custom elements.

A notable change included in this upstreaming is that the order of
attributes created by the parser is now well-defined. See
WICG/webcomponents#474 for discussion, as well
as https://www.w3.org/Bugs/Public/show_bug.cgi?id=17871.
"custom tag" becomes "autonomous custom element", and "type extension"
becomes "customized built-in element".

Fixes WICG/webcomponents#434.
@domenic domenic force-pushed the customelements branch from 3161de2 to 1d596a5 Apr 13, 2016
@domenic domenic merged commit 1d596a5 into master Apr 13, 2016
@annevk
Copy link
Member

@annevk annevk commented Apr 14, 2016

\o/

(Do we want to keep this branch alive as we did in DOM?)

@domenic
Copy link
Member Author

@domenic domenic commented Apr 14, 2016

Yeah, we probably should, and I can also help maintain a similar wiki page.

@annevk
Copy link
Member

@annevk annevk commented Apr 14, 2016

Protected.

domenic added a commit that referenced this issue Apr 20, 2016
Fixes #1015, and matches all non-Gecko engines. Before #1012 the spec
was unclear on this; in #1012 the spec sided with Gecko, but per #1015
it is better to side with the majority.
annevk added a commit that referenced this issue Apr 22, 2016
Fixes #1015, and matches all non-Gecko engines. Before #1012 the standard was unclear on this; in #1012 the standard sided with Gecko, but per #1015 it is better to side with the majority.
@annevk annevk deleted the customelements branch Aug 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants