WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
137137
Stop using legacy NODE_TYPE_CASTS() macro for HTML Elements
https://bugs.webkit.org/show_bug.cgi?id=137137
Summary
Stop using legacy NODE_TYPE_CASTS() macro for HTML Elements
Chris Dumez
Reported
2014-09-25 19:55:16 PDT
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.
Attachments
WIP Patch
(130.48 KB, patch)
2014-09-25 21:39 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(151.19 KB, patch)
2014-09-25 22:02 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(155.01 KB, patch)
2014-09-25 22:24 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(155.61 KB, patch)
2014-09-26 07:39 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2
(1.09 MB, application/zip)
2014-09-26 09:02 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
(618.09 KB, application/zip)
2014-09-26 09:12 PDT
,
Build Bot
no flags
Details
Patch
(155.66 KB, patch)
2014-09-26 13:31 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(157.14 KB, patch)
2014-09-26 15:51 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(157.62 KB, patch)
2014-09-26 17:49 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2014-09-25 21:39:14 PDT
Created
attachment 238695
[details]
WIP Patch
WebKit Commit Bot
Comment 2
2014-09-25 21:40:41 PDT
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.
Chris Dumez
Comment 3
2014-09-25 22:02:16 PDT
Created
attachment 238696
[details]
Patch
WebKit Commit Bot
Comment 4
2014-09-25 22:04:21 PDT
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.
Chris Dumez
Comment 5
2014-09-25 22:24:18 PDT
Created
attachment 238697
[details]
Patch
WebKit Commit Bot
Comment 6
2014-09-25 22:25:57 PDT
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.
Chris Dumez
Comment 7
2014-09-26 07:39:04 PDT
Created
attachment 238714
[details]
Patch
WebKit Commit Bot
Comment 8
2014-09-26 07:41:06 PDT
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.
Build Bot
Comment 9
2014-09-26 09:02:13 PDT
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
Build Bot
Comment 10
2014-09-26 09:02:18 PDT
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
Build Bot
Comment 11
2014-09-26 09:12:43 PDT
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
Build Bot
Comment 12
2014-09-26 09:12:48 PDT
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
Chris Dumez
Comment 13
2014-09-26 13:31:27 PDT
Created
attachment 238731
[details]
Patch
WebKit Commit Bot
Comment 14
2014-09-26 13:32:41 PDT
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.
Benjamin Poulain
Comment 15
2014-09-26 14:59:01 PDT
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
Chris Dumez
Comment 16
2014-09-26 15:33:59 PDT
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.
Chris Dumez
Comment 17
2014-09-26 15:51:30 PDT
Created
attachment 238744
[details]
Patch
WebKit Commit Bot
Comment 18
2014-09-26 15:53:02 PDT
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.
Benjamin Poulain
Comment 19
2014-09-26 17:36:51 PDT
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.
Chris Dumez
Comment 20
2014-09-26 17:49:31 PDT
Created
attachment 238752
[details]
Patch
WebKit Commit Bot
Comment 21
2014-09-26 17:51:49 PDT
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.
WebKit Commit Bot
Comment 22
2014-09-26 18:32:45 PDT
Comment on
attachment 238752
[details]
Patch Clearing flags on attachment: 238752 Committed
r174031
: <
http://trac.webkit.org/changeset/174031
>
WebKit Commit Bot
Comment 23
2014-09-26 18:32:51 PDT
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.
Top of Page
Format For Printing
XML
Clone This Bug