Bug 153092 - Add document.defineCustomElement
Summary: Add document.defineCustomElement
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:
Depends on:
Blocks: 150225
  Show dependency treegraph
 
Reported: 2016-01-13 23:39 PST by Ryosuke Niwa
Modified: 2016-01-14 18:59 PST (History)
7 users (show)

See Also:


Attachments
Adds the feature (43.44 KB, patch)
2016-01-14 00:11 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Added the missing expected results (45.06 KB, patch)
2016-01-14 00:13 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-yosemite (868.62 KB, application/zip)
2016-01-14 01:12 PST, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (1.03 MB, application/zip)
2016-01-14 01:16 PST, Build Bot
no flags Details
Fixed a failing test (45.07 KB, patch)
2016-01-14 14:04 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-01-13 23:39:39 PST
Add a method to define a custom element and allow subclassing of HTMLElement.
Comment 1 Ryosuke Niwa 2016-01-14 00:11:31 PST
Created attachment 268944 [details]
Adds the feature
Comment 2 Ryosuke Niwa 2016-01-14 00:13:55 PST
Created attachment 268945 [details]
Added the missing expected results
Comment 3 Ryosuke Niwa 2016-01-14 00:15:33 PST
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.
Comment 4 Build Bot 2016-01-14 01:12:44 PST
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
Comment 5 Build Bot 2016-01-14 01:12:46 PST
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
Comment 6 Build Bot 2016-01-14 01:16:07 PST
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
Comment 7 Build Bot 2016-01-14 01:16:11 PST
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
Comment 8 Ryosuke Niwa 2016-01-14 14:04:24 PST
Created attachment 269001 [details]
Fixed a failing test
Comment 9 Chris Dumez 2016-01-14 16:07:44 PST
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
Comment 10 Ryosuke Niwa 2016-01-14 18:59:15 PST
Committed r195087: <http://trac.webkit.org/changeset/195087>