The spec has updated the list of element on which attachShadow is allowed. We should update our implementation accordingly to avoid the future compatibility issues.
*** Bug 157928 has been marked as a duplicate of this bug. ***
Created attachment 280320 [details] Patch
Attachment 280320 [details] did not pass style-queue: ERROR: Source/WebCore/dom/Document.cpp:1129: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 280507 [details] Fixed GTK/Windows builds
Attachment 280507 [details] did not pass style-queue: ERROR: Source/WebCore/dom/Document.cpp:1129: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 280509 [details] Another build fix attempt
Attachment 280509 [details] did not pass style-queue: ERROR: Source/WebCore/dom/Document.cpp:1129: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 280509 [details] Another build fix attempt View in context: https://bugs.webkit.org/attachment.cgi?id=280509&action=review Did you change all the headers that you touched to pragma once? Would be nice to do that. > Source/WebCore/dom/Document.cpp:1124 > + for (unsigned i = 0; i < localName.length(); i++) { > + if (isASCIIUpper(localName[i])) > + return CustomElementNameValidationStatus::ContainsUpperCase; > + if (localName[i] == '-') > + containsHyphen = true; > + } More efficient not to call length() over and over again and index into the string over and over again. Could use a couple local variables, or alternatively something like the StringView::codeUnits() iterator. > Source/WebCore/dom/Document.cpp:1138 > +#if ENABLE(MATHML) > + || localName == MathMLNames::annotation_xmlTag.localName() > +#endif Seems like we might want to check this even if MathML was not turned on. > Source/WebCore/dom/Element.cpp:1714 > + static NeverDestroyed<HashSet<AtomicString>> tagNames; > + if (tagNames.get().isEmpty()) { > + const AtomicString nameList[] = { > + articleTag.localName(), > + asideTag.localName(), > + blockquoteTag.localName(), > + bodyTag.localName(), > + divTag.localName(), > + footerTag.localName(), > + h1Tag.localName(), > + h2Tag.localName(), > + h3Tag.localName(), > + h4Tag.localName(), > + h5Tag.localName(), > + h6Tag.localName(), > + headerTag.localName(), > + navTag.localName(), > + pTag.localName(), > + sectionTag.localName(), > + spanTag.localName() > + }; > + for (auto& name : nameList) > + tagNames.get().add(name); > + } This is not the best idiom, because we end up checking if the set is empty every time. Also, we don’t want an array of reference counted local names. Instead we want an array of tags, ideally something that can be initialized without any code running. Something like this: static NeverDestroyed<HashSet<AtomicString>> tagNames = [] { HashSet<AtomicString> set; const HTMLQualifiedName& const tagList[] = { articleTag, ... }; for (auto& tag : tagList) set.add(tag()); return set; }; > Source/WebCore/dom/Element.cpp:1717 > + if (element.namespaceURI() != xhtmlNamespaceURI) > + return false; I believe it’s more efficient to just check is<HTMLElement>, and I am pretty sure it accomplishes the same thing. > Source/WebCore/dom/Element.cpp:1719 > + return tagNames.get().contains(element.localName()) || Document::validateCustomElementName(element.localName()) == CustomElementNameValidationStatus::Valid; I suggest putting element.localName() into a local variable.
(In reply to comment #8) > const HTMLQualifiedName& const tagList[] = { Actually I think this: static const HTMLQualifiedName& const tagList[] = {
Committed r201739: <http://trac.webkit.org/changeset/201739>
<rdar://problem/26666315>