Skip to content

Add missing [HTMLConstructor] annotations#2032

Merged
domenic merged 5 commits intomasterfrom
missing-htmlconstructors
Nov 16, 2016
Merged

Add missing [HTMLConstructor] annotations#2032
domenic merged 5 commits intomasterfrom
missing-htmlconstructors

Conversation

@domenic
Copy link
Member

@domenic domenic commented Nov 9, 2016

Fixes #2028. [HTMLConstructor] was introduced in #1404, and these
constructors were accidentally left out of the fun.


For tests, this is part of the larger testing program of customized built-in elements, which implementers are working on contributing to.

Fixes #2028. [HTMLConstructor] was introduced in #1404, and these
constructors were accidentally left out of the fun.
@domenic domenic added the topic: custom elements Relates to custom elements (as defined in DOM and HTML) label Nov 9, 2016
@annevk
Copy link
Member

annevk commented Nov 10, 2016

Should the note not also be added to https://html.spec.whatwg.org/multipage/dom.html#html-element-constructors since a couple of element names can end up using the HTMLUnknownElement class?

@domenic
Copy link
Member Author

domenic commented Nov 10, 2016

Hmm interesting. Probably not there, but maybe something in https://html.spec.whatwg.org/multipage/scripting.html#custom-elements-customized-builtin-example saying e.g.

Customized built-in elements can only extend existing HTML element types defined in this specification that have a valid HTML element constructor. Notably, the types bgsound, blink, isindex, multicol, nextid, and spacer cannot be extended, since their element interface is HTMLUnknownElement.

@domenic domenic added the do not merge yet Pull request must not be merged per rationale in comment label Nov 10, 2016
@annevk
Copy link
Member

annevk commented Nov 10, 2016

I guess. We should also add to that note the reason that only existing HTML elements can be extended. The being forward compatible argument.

@domenic domenic removed the do not merge yet Pull request must not be merged per rationale in comment label Nov 10, 2016
@annevk annevk added the do not merge yet Pull request must not be merged per rationale in comment label Nov 11, 2016
@annevk
Copy link
Member

annevk commented Nov 11, 2016

Adding back the keyword for now since you asked a question at #2028 (comment) that seems unresolved. Looks good to me otherwise.

@domenic domenic removed the do not merge yet Pull request must not be merged per rationale in comment label Nov 15, 2016
@domenic
Copy link
Member Author

domenic commented Nov 15, 2016

I made applet/keygen uncustomizable, with an explanation; /cc @dominiccooney @smaug----. Seems like the way to go.

<pre class="idl">interface <dfn>HTMLKeygenElement</dfn> : <span>HTMLElement</span> { // Note: <a href="#customized-built-in-element-restrictions">intentionally</a> not [<span>HTMLConstructor</span>]
attribute boolean <span data-x="dom-fe-autofocus">autofocus</span>;
attribute DOMString <span data-x="dom-keygen-challenge">challenge</span>;
attribute boolean <span data-x="dom-fe-disabled">disabled</span>;
Copy link
Member

Choose a reason for hiding this comment

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

I think typically the IDL blocks are indented such that 'attribute' lines up with 'readonly attribute' (when there are no extended attributes), so 9 more spaces before 'attribute'. (Same for HTMLAppletElement.)

Otherwise LGTM

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't seem to do that anywhere else in the spec that I can find.

source Outdated
[<span>CEReactions</span>] attribute boolean <span data-x="dom-fe-autofocus">autofocus</span>;
[<span>CEReactions</span>] attribute DOMString <span data-x="dom-keygen-challenge">challenge</span>;
[<span>CEReactions</span>] attribute boolean <span data-x="dom-fe-disabled">disabled</span>;
<pre class="idl">interface <dfn>HTMLKeygenElement</dfn> : <span>HTMLElement</span> { // Note: <a href="#customized-built-in-element-restrictions">intentionally</a> not [<span>HTMLConstructor</span>]
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be nice if the note was on its own line preceding the interface just as you did above.

source Outdated
[<span>CEReactions</span>] attribute USVString _<span data-x="dom-applet-object">object</span>; // the underscore is not part of the identifier <!-- it's a Web IDL escaping mechanism -->
[<span>CEReactions</span>] attribute unsigned long <span data-x="dom-applet-vspace">vspace</span>;
[<span>CEReactions</span>] attribute DOMString <span data-x="dom-applet-width">width</span>;
<pre class="idl">interface <dfn>HTMLAppletElement</dfn> : <span>HTMLElement</span> { // Note: <a href="#customized-built-in-element-restrictions">intentionally</a> not [<span>HTMLConstructor</span>]
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

@domenic domenic merged commit 7d6b279 into master Nov 16, 2016
@domenic domenic deleted the missing-htmlconstructors branch November 16, 2016 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: custom elements Relates to custom elements (as defined in DOM and HTML)

Development

Successfully merging this pull request may close these issues.

3 participants