Stop using legacy NODE_TYPE_CASTS() macro for HTML Elements and use the new SPECIALIZE_TYPE_TRAITS_*() macro instead so that is<>() / downcast<>() works for those types.
Created attachment 238695 [details] WIP Patch
Attachment 238695 [details] did not pass style-queue: ERROR: Source/WebCore/html/HTMLTableCellElement.h:67: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTableSectionElement.h:66: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLMediaElement.h:915: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTextFormControlElement.h:156: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLPlugInImageElement.h:162: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLFrameElementBase.h:81: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 6 in 94 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 238696 [details] Patch
Attachment 238696 [details] did not pass style-queue: ERROR: Source/WebCore/html/HTMLTableCellElement.h:67: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTableSectionElement.h:66: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLMediaElement.h:915: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTextFormControlElement.h:156: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLPlugInImageElement.h:162: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLFrameElementBase.h:81: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 6 in 97 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 238697 [details] Patch
Attachment 238697 [details] did not pass style-queue: ERROR: Source/WebCore/html/HTMLTableCellElement.h:67: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTableSectionElement.h:66: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLMediaElement.h:915: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTextFormControlElement.h:156: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLPlugInImageElement.h:162: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLFrameElementBase.h:81: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 6 in 97 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 238714 [details] Patch
Attachment 238714 [details] did not pass style-queue: ERROR: Source/WebCore/html/HTMLTableCellElement.h:67: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTableSectionElement.h:66: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLMediaElement.h:915: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTextFormControlElement.h:156: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLPlugInImageElement.h:162: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLFrameElementBase.h:81: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 6 in 97 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 238714 [details] Patch Attachment 238714 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4672407436525568 New failing tests: editing/unsupported-content/list-type-before.html editing/style/push-down-font-styles-win.html editing/style/fore-color-by-name.html editing/pasteboard/4744008.html editing/unsupported-content/table-type-before.html editing/unsupported-content/list-delete-003.html editing/unsupported-content/list-delete-001.html editing/unsupported-content/list-type-after.html editing/unsupported-content/table-delete-002.html editing/style/push-down-font-styles-mac.html editing/unsupported-content/table-type-after.html editing/pasteboard/insert-font-with-size-and-css.html editing/style/inline-style-container.html editing/style/inline-style-extend-run.html
Created attachment 238720 [details] Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-12 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 238714 [details] Patch Attachment 238714 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6201980582100992 New failing tests: editing/style/push-down-font-styles-win.html editing/style/fore-color-by-name.html editing/pasteboard/4744008.html editing/style/push-down-font-styles-mac.html editing/pasteboard/insert-font-with-size-and-css.html editing/style/inline-style-container.html editing/style/inline-style-extend-run.html
Created attachment 238721 [details] Archive of layout-test-results from webkit-ews-04 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 238731 [details] Patch
Attachment 238731 [details] did not pass style-queue: ERROR: Source/WebCore/html/HTMLTableCellElement.h:67: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTableSectionElement.h:66: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLMediaElement.h:915: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTextFormControlElement.h:156: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLPlugInImageElement.h:162: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLFrameElementBase.h:81: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 6 in 97 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 238731 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238731&action=review > Source/WebCore/bindings/js/JSPluginElementFunctions.cpp:130 > - return 0; > + return false; Uh! > Source/WebCore/css/StyleResolver.cpp:454 > return 0; nullptr! > Source/WebCore/css/StyleResolver.cpp:456 > return 0; nullptr! > Source/WebCore/html/HTMLElement.cpp:871 > + return is<HTMLElement>(node) && (downcast<HTMLElement>(node).hasTagName(bdiTag) || downcast<HTMLElement>(node).fastHasAttribute(dirAttr)); Why not is<HTMLBDIElement>? > Source/WebCore/html/HTMLPlugInImageElement.cpp:-419 > - if (node->isPluginElement()) { > - HTMLPlugInElement* plugInElement = toHTMLPlugInElement(node); > - if (plugInElement->isPlugInImageElement()) { This code was weird. > Source/WebCore/html/HTMLSelectElement.cpp:452 > + before = downcast<HTMLElement>(options()->item(index+1)); index + 1 > Source/WebCore/html/HTMLSelectElement.cpp:474 > + add(downcast<HTMLElement>(option.get()), 0, ec); 0 -> nullptr > Source/WebCore/html/LabelableElement.h:55 > +inline bool isLabelableElement(const Node& node) { return node.isHTMLElement() && downcast<HTMLElement>(node).isLabelable(); } node.isHTMLElement() -> is<HTMLElement>(node) > Source/WebCore/html/RangeInputType.cpp:271 > - return &toHTMLElement(*element().userAgentShadowRoot()->firstChild()->firstChild()); > + return &downcast<HTMLElement>(*element().userAgentShadowRoot()->firstChild()->firstChild()); Uh, what. > Source/WebCore/html/shadow/MediaControlElementTypes.cpp:64 > ASSERT_WITH_SECURITY_IMPLICATION(node->isMediaControlElement()); > - HTMLElement* element = toHTMLElement(node); > + HTMLElement* element = downcast<HTMLElement>(node); Hum, this looks fragile. > Source/WebCore/html/shadow/TextControlInnerElements.h:74 > +inline bool isTextControlInnerTextElement(const Node& node) { return node.isHTMLElement() && isTextControlInnerTextElement(downcast<HTMLElement>(node)); } node.isHTMLElement() -> is<HTMLElement>(node) > Source/WebCore/loader/archive/cf/LegacyWebArchive.cpp:520 > + if ((is<HTMLFrameElementBase>(node) || is<HTMLObjectElement>(node)) Isn't this is<HTMLFrameOwnerElement>(node)? > Source/WebCore/mathml/MathMLElement.cpp:87 > - if (node.isMathMLElement()) { > - auto& mathmlElement = downcast<MathMLElement>(node); > - return mathmlElement.hasTagName(MathMLNames::mathTag); > - } > + if (is<MathMLMathElement>(node)) > + return true; This isn't strictly equivalent. Previously, the remaining of the function would not be evaluated if node is a mathMLElement. > Source/WebCore/mathml/MathMLElement.cpp:90 > - if (node.isSVGElement()) { > - auto& svgElement = downcast<SVGElement>(node); > - return svgElement.hasTagName(SVGNames::svgTag); > - } > + if (is<SVGSVGElement>(node)) > + return true; ditto. > Source/WebCore/mathml/MathMLElement.cpp:276 > - if (child.isMathMLElement() && (MathMLSelectElement::isMathMLEncoding(value) || MathMLSelectElement::isHTMLEncoding(value))) { > - auto& mathmlElement = downcast<MathMLElement>(child); > - return mathmlElement.hasTagName(MathMLNames::mathTag); > - } > + if (is<MathMLMathElement>(child) && (MathMLSelectElement::isMathMLEncoding(value) || MathMLSelectElement::isHTMLEncoding(value))) > + return true; > > - if (child.isSVGElement() && (MathMLSelectElement::isSVGEncoding(value) || MathMLSelectElement::isHTMLEncoding(value))) { > - auto& svgElement = downcast<SVGElement>(child); > - return svgElement.hasTagName(SVGNames::svgTag); > - } > + if (is<SVGSVGElement>(child) && (MathMLSelectElement::isSVGEncoding(value) || MathMLSelectElement::isHTMLEncoding(value))) > + return true; > > - if (child.isHTMLElement() && MathMLSelectElement::isHTMLEncoding(value)) { > - auto& htmlElement = toHTMLElement(child); > - return htmlElement.hasTagName(HTMLNames::htmlTag) || (isFlowContent(htmlElement) && StyledElement::childShouldCreateRenderer(child)); > + if (is<HTMLElement>(child) && MathMLSelectElement::isHTMLEncoding(value)) { > + auto& htmlElement = downcast<HTMLElement>(child); > + return is<HTMLHtmlElement>(htmlElement) || (isFlowContent(htmlElement) && StyledElement::childShouldCreateRenderer(child)); > } etc
Comment on attachment 238731 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238731&action=review >> Source/WebCore/bindings/js/JSPluginElementFunctions.cpp:130 >> + return false; > > Uh! Yes, the function returns a boolean. >> Source/WebCore/css/StyleResolver.cpp:454 >> return 0; > > nullptr! Ok. >> Source/WebCore/css/StyleResolver.cpp:456 >> return 0; > > nullptr! Ok. >> Source/WebCore/html/HTMLElement.cpp:871 >> + return is<HTMLElement>(node) && (downcast<HTMLElement>(node).hasTagName(bdiTag) || downcast<HTMLElement>(node).fastHasAttribute(dirAttr)); > > Why not is<HTMLBDIElement>? Ok. >> Source/WebCore/html/HTMLSelectElement.cpp:452 >> + before = downcast<HTMLElement>(options()->item(index+1)); > > index + 1 Ok. >> Source/WebCore/html/HTMLSelectElement.cpp:474 >> + add(downcast<HTMLElement>(option.get()), 0, ec); > > 0 -> nullptr Ok. >> Source/WebCore/html/LabelableElement.h:55 >> +inline bool isLabelableElement(const Node& node) { return node.isHTMLElement() && downcast<HTMLElement>(node).isLabelable(); } > > node.isHTMLElement() -> is<HTMLElement>(node) Ok. >> Source/WebCore/html/RangeInputType.cpp:271 >> + return &downcast<HTMLElement>(*element().userAgentShadowRoot()->firstChild()->firstChild()); > > Uh, what. lol, Ok. >> Source/WebCore/html/shadow/MediaControlElementTypes.cpp:64 >> + HTMLElement* element = downcast<HTMLElement>(node); > > Hum, this looks fragile. Ok. I'll remove the cast. >> Source/WebCore/html/shadow/TextControlInnerElements.h:74 >> +inline bool isTextControlInnerTextElement(const Node& node) { return node.isHTMLElement() && isTextControlInnerTextElement(downcast<HTMLElement>(node)); } > > node.isHTMLElement() -> is<HTMLElement>(node) Ok. >> Source/WebCore/loader/archive/cf/LegacyWebArchive.cpp:520 >> + if ((is<HTMLFrameElementBase>(node) || is<HTMLObjectElement>(node)) > > Isn't this is<HTMLFrameOwnerElement>(node)? No, HTMLFrameOwnerElement also covers HTMLAppletElement and HTMLEmbedElement. >> Source/WebCore/mathml/MathMLElement.cpp:87 >> + return true; > > This isn't strictly equivalent. > > Previously, the remaining of the function would not be evaluated if node is a mathMLElement. Ok, good catch. >> Source/WebCore/mathml/MathMLElement.cpp:90 >> + return true; > > ditto. Ok. >> Source/WebCore/mathml/MathMLElement.cpp:276 >> } > > etc Ok.
Created attachment 238744 [details] Patch
Attachment 238744 [details] did not pass style-queue: ERROR: Source/WebCore/html/HTMLTableCellElement.h:67: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTableSectionElement.h:66: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLMediaElement.h:915: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTextFormControlElement.h:156: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLPlugInImageElement.h:162: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLFrameElementBase.h:81: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 6 in 97 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 238744 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238744&action=review > Source/WebCore/page/Frame.cpp:397 > if (n->hasTagName(tdTag) && !startingTableCell) { > - startingTableCell = toHTMLTableCellElement(n); > + startingTableCell = downcast<HTMLTableCellElement>(n); > } else if (n->hasTagName(trTag) && startingTableCell) { Maybe take the opportunity to fix the style here? > Source/WebCore/page/SpatialNavigation.cpp:-762 > -}; Weird, I thought the compiler had a warning for this.
Created attachment 238752 [details] Patch
Attachment 238752 [details] did not pass style-queue: ERROR: Source/WebCore/html/HTMLTableCellElement.h:67: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTableSectionElement.h:66: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLMediaElement.h:915: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLTextFormControlElement.h:156: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLPlugInImageElement.h:162: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/html/HTMLFrameElementBase.h:81: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 6 in 97 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 238752 [details] Patch Clearing flags on attachment: 238752 Committed r174031: <http://trac.webkit.org/changeset/174031>
All reviewed patches have been landed. Closing bug.