Bug 157706

*** 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>