Bug 155010 - Update defineCustomElement according to the spec rewrite
Summary: Update defineCustomElement according to the spec rewrite
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks: 154907
  Show dependency treegraph
 
Reported: 2016-03-03 22:36 PST by Ryosuke Niwa
Modified: 2016-03-04 17:23 PST (History)
8 users (show)

See Also:


Attachments
Fixes the bug (51.59 KB, patch)
2016-03-03 22:53 PST, Ryosuke Niwa
cdumez: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2016-03-03 22:36:07 PST
Rename document.defineCustomElement to document.defineElement
and disallow a single class from defining multiple custom elements.

Also remove the optional first argument from the HTMLElement constructor.
Comment 1 Radar WebKit Bug Importer 2016-03-03 22:37:02 PST
<rdar://problem/24970878>
Comment 2 Ryosuke Niwa 2016-03-03 22:53:42 PST
Created attachment 272835 [details]
Fixes the bug
Comment 3 Chris Dumez 2016-03-04 15:45:44 PST
Comment on attachment 272835 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=272835&action=review

r=me with questions

> Source/WebCore/dom/CustomElementDefinitions.h:55
> +    bool containsInterface(const JSC::JSObject*) const;

Would naming this "containsConstructor" make more sense?

> Source/WebCore/dom/Document.idl:298
> +    void defineElement(DOMString tagName, CustomElementInterface elementInterface);

The spec has:
void defineElement(DOMString type, Function constructor, optional ElementRegistrationOptions options);

Why the mismatch?
Comment 4 Ryosuke Niwa 2016-03-04 17:15:08 PST
Comment on attachment 272835 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=272835&action=review

Thanks for the review!

>> Source/WebCore/dom/CustomElementDefinitions.h:55
>> +    bool containsInterface(const JSC::JSObject*) const;
> 
> Would naming this "containsConstructor" make more sense?

Renamed.

>> Source/WebCore/dom/Document.idl:298
>> +    void defineElement(DOMString tagName, CustomElementInterface elementInterface);
> 
> The spec has:
> void defineElement(DOMString type, Function constructor, optional ElementRegistrationOptions options);
> 
> Why the mismatch?

Changed it to:

void defineElement(DOMString localName, Function constructor);

since we don't support the optional options argument which is used to subclass subclasses of HTMLElement; e.g. HTMLInputElement.
I've also filed a Github issue to rename the first argument to localName: https://github.com/w3c/webcomponents/issues/416
since type is too generic name and doesn't match the argument name of createElement.
Comment 5 Ryosuke Niwa 2016-03-04 17:23:31 PST
Committed r197602: <http://trac.webkit.org/changeset/197602>