Bug 164700

Summary:

document.createElementNS doesn't construct a custom element

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

DOM

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, commit-queue, dbates, esprehn+autocc, kangil.han, koivisto, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

154907

Attachments:

Description	Flags
Fixes the bug	none
Added a change log description	none
Updated for ToT	none
Patch for landing	none
Patch for landing	none

Ryosuke Niwa

Reported 2016-11-13 00:15:00 PST

document.createElementNS doesn't create a custom element :(

Attachments
Fixes the bug (19.96 KB, patch) 2016-11-13 00:56 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Added a change log description (20.06 KB, patch) 2016-11-13 00:58 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Updated for ToT (20.02 KB, patch) 2016-11-13 01:09 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Patch for landing (20.18 KB, patch) 2016-11-14 15:53 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Patch for landing (20.17 KB, patch) 2016-11-14 15:54 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Ryosuke Niwa

Comment 1 2016-11-13 00:56:45 PST

Created attachment 294659 [details] Fixes the bug

Ryosuke Niwa

Comment 2 2016-11-13 00:58:16 PST

Created attachment 294660 [details] Added a change log description

Radar WebKit Bug Importer

Comment 3 2016-11-13 00:59:41 PST

<rdar://problem/29237299>

Radar WebKit Bug Importer

Comment 4 2016-11-13 01:01:06 PST

<rdar://problem/29237304>

Ryosuke Niwa

Comment 5 2016-11-13 01:09:31 PST

Created attachment 294661 [details] Updated for ToT

Darin Adler

Comment 6 2016-11-13 11:34:02 PST

Comment on attachment 294661 [details] Updated for ToT View in context: https://bugs.webkit.org/attachment.cgi?id=294661&action=review > Source/WebCore/bindings/js/JSCustomElementInterface.cpp:85 > + return element.get(); This churns the reference count. Should do WTFMove(element) instead to avoid that. > Source/WebCore/dom/DOMImplementation.cpp:118 > + ASSERT(!document->domWindow()); // createElemenNS should not synchronously construct a custom element here. Typo, missing "t" in createElementNS. Also completely unclear what domWindow has to do with createElementNS. > Source/WebCore/dom/Document.cpp:881 > +static bool isValidHTMLElementName(const AtomicString& localName) { return Document::isValidName(localName); } > +static bool isValidHTMLElementName(const QualifiedName& name) { return Document::isValidName(name.localName()); } Can we write these out normally instead of as one-liners? Should they be marked inline? > Source/WebCore/dom/Document.cpp:883 > +template <class NameType> Should be template<typename NameType>; we prefer typename or class and no space after "template".

Ryosuke Niwa

Comment 7 2016-11-14 15:53:00 PST

Thanks for the review. (In reply to comment #6) > Comment on attachment 294661 [details] > Updated for ToT > > View in context: > https://bugs.webkit.org/attachment.cgi?id=294661&action=review > > > Source/WebCore/bindings/js/JSCustomElementInterface.cpp:85 > > + return element.get(); > > This churns the reference count. Should do WTFMove(element) instead to avoid > that. Fixed. > > Source/WebCore/dom/DOMImplementation.cpp:118 > > + ASSERT(!document->domWindow()); // createElemenNS should not synchronously construct a custom element here. > > Typo, missing "t" in createElementNS. > > Also completely unclear what domWindow has to do with createElementNS. Revised to: If domWindow is not null, createElementNS could find CustomElementRegistry and arbitrary scripts. > > Source/WebCore/dom/Document.cpp:881 > > +static bool isValidHTMLElementName(const AtomicString& localName) { return Document::isValidName(localName); } > > +static bool isValidHTMLElementName(const QualifiedName& name) { return Document::isValidName(name.localName()); } > > Can we write these out normally instead of as one-liners? Should they be > marked inline? Fixed, and added inline keyword. > > Source/WebCore/dom/Document.cpp:883 > > +template <class NameType> > > Should be template<typename NameType>; we prefer typename or class and no > space after "template". Fixed.

Ryosuke Niwa

Comment 8 2016-11-14 15:53:42 PST

Created attachment 294767 [details] Patch for landing

Ryosuke Niwa

Comment 9 2016-11-14 15:54:23 PST

Created attachment 294768 [details] Patch for landing

WebKit Commit Bot

Comment 10 2016-11-14 16:30:10 PST

Comment on attachment 294768 [details] Patch for landing Clearing flags on attachment: 294768 Committed r208716: <http://trac.webkit.org/changeset/208716>

WebKit Commit Bot

Comment 11 2016-11-14 16:30:16 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.