Bug 155010

Summary: Update defineCustomElement according to the spec rewrite
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, esprehn+autocc, gyuyoung.kim, kangil.han, koivisto, kondapallykalyan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 154907    
Attachments:
Description Flags
Fixes the bug cdumez: review+

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>