Bug 153092 – Add document.defineCustomElement

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Ryosuke Niwa]

Add a method to define a custom element and allow subclassing of HTMLElement.

[Ryosuke Niwa]

Created attachment 268944 [details]
Adds the feature

[Ryosuke Niwa]

Created attachment 268945 [details]
Added the missing expected results

[Ryosuke Niwa]

This patch doesn't add support for creating a custom element in document.createElement, HTML parser, etc... as well as lifecycle callbacks. They're coming in separate patches.

[Build Bot]

Comment on attachment 268945 [details]
Added the missing expected results

Attachment 268945 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/689451

New failing tests:
imported/w3c/web-platform-tests/html/dom/interfaces.html

[Build Bot]

Created attachment 268948 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 268945 [details]
Added the missing expected results

Attachment 268945 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/689452

New failing tests:
imported/w3c/web-platform-tests/html/dom/interfaces.html

[Build Bot]

Created attachment 268949 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Ryosuke Niwa]

Created attachment 269001 [details]
Fixed a failing test

[Chris Dumez]

Comment on attachment 269001 [details]
Fixed a failing test

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

r=me with comments.

> Source/WebCore/bindings/js/JSDocumentCustom.cpp:161
> +        setDOMException(&state, NOT_SUPPORTED_ERR);

We could probably provide a meaningful error message here.

> Source/WebCore/bindings/js/JSHTMLElementCustom.cpp:42
> +    DOMConstructorObject* jsConstructor = jsCast<DOMConstructorObject*>(state->callee());

auto*

> Source/WebCore/bindings/js/JSHTMLElementCustom.cpp:44
> +    ScriptExecutionContext* context = jsConstructor->scriptExecutionContext();

auto*

> Source/WebCore/bindings/js/JSHTMLElementCustom.cpp:45
> +    if (!context || !context->isDocument())

if (!is<Document>(context))

> Source/WebCore/bindings/js/JSHTMLElementCustom.cpp:58
> +        if (state->argumentCount() < 1)

UNLIKELY()

> Source/WebCore/bindings/js/JSHTMLElementCustom.cpp:80
> +    if (state->hadException())

UNLIKELY()

> Source/WebCore/bindings/js/JSHTMLElementCustom.cpp:84
> +    auto* jsElement = JSHTMLElement::create(newElementStructure, globalObject, element.get());

I don't think you need the .get()

> Source/WebCore/dom/CustomElementDefinitions.cpp:53
> +    // FIXME: We should be taking the advantage of QualifiedNames in SVG and MathML.

This does not sound like a lot of work to do in this patch and would be a lot more efficient as we would just compare AtomicStrings.

> Source/WebCore/dom/CustomElementDefinitions.cpp:69
> +    auto* constructor = interface->constructor();

I would move this after the assertion.

> Source/WebCore/dom/CustomElementDefinitions.cpp:71
> +    ASSERT(m_nameMap.find(fullName.localName()) == m_nameMap.end());

I would just use !contains() to make this shorter.

> Source/WebCore/dom/CustomElementDefinitions.cpp:74
> +    auto it = m_constructorMap.add(constructor, fullName);

We usually call this one addResult rather than "it" I believe? The iterator is a member of the AddResult.

> Source/WebCore/dom/CustomElementDefinitions.h:48
> +class CustomElementDefinitions {

WTF_MAKE_FAST_ALLOCATED

> LayoutTests/fast/custom-elements/Document-defineCustomElement.html:29
> +    assert_throws({'name': 'SyntaxError'}, function () { document.defineCustomElement('aBc', MyCustomElement); },

I would add a '-' in there so it is otherwise valid.

> LayoutTests/fast/custom-elements/HTMLElement-constructor.html:43
> +    class CustomElmeentWithInferredTagName extends HTMLElement {};

Typo: Elmeent

[Ryosuke Niwa]

Committed r195087: <http://trac.webkit.org/changeset/195087>