RESOLVED FIXED 164700
document.createElementNS doesn't construct a custom element
https://bugs.webkit.org/show_bug.cgi?id=164700
Summary document.createElementNS doesn't construct a custom element
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
Added a change log description (20.06 KB, patch)
2016-11-13 00:58 PST, Ryosuke Niwa
no flags
Updated for ToT (20.02 KB, patch)
2016-11-13 01:09 PST, Ryosuke Niwa
no flags
Patch for landing (20.18 KB, patch)
2016-11-14 15:53 PST, Ryosuke Niwa
no flags
Patch for landing (20.17 KB, patch)
2016-11-14 15:54 PST, Ryosuke Niwa
no flags
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
Radar WebKit Bug Importer
Comment 4 2016-11-13 01:01:06 PST
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.