Summary: | Use childrenOfType<> more in HTMLTableElement | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> | ||||||||
Component: | DOM | Assignee: | Anne van Kesteren <annevk> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, darin, esprehn+autocc, gyuyoung.kim, rniwa, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Gyuyoung Kim
2013-11-20 01:23:51 PST
Created attachment 217406 [details]
Patch
Darin, as you said https://bugs.webkit.org/show_bug.cgi?id=124571#c4, this patch simplified a loop to find a first children by using childrenOfType<>. However, "childrenOfType<HTMLTableCaptionElement>(*this).first()" returns const element. So, I have to use const_cast to remove constness. Do you have better idea ? Comment on attachment 217406 [details] Patch Attachment 217406 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/30268003 New failing tests: dom/html/level2/html/HTMLCollection06.html dom/xhtml/level2/html/HTMLTableElement21.xhtml dom/html/level2/html/HTMLTableElement28.html dom/html/level2/html/HTMLTableElement22.html dom/html/level2/html/HTMLTableRowElement01.html accessibility/table-one-cell.html dom/html/level2/html/HTMLTableElement24.html dom/html/level2/html/HTMLTableElement30.html dom/html/level2/html/table07.html dom/xhtml/level2/html/HTMLCollection07.xhtml dom/xhtml/level2/html/HTMLTableElement19.xhtml dom/html/level2/html/HTMLTableElement04.html dom/html/level2/html/HTMLTableElement06.html dom/html/level2/html/HTMLTableElement21.html dom/xhtml/level2/html/HTMLTableElement22.xhtml accessibility/table-detection.html dom/html/level2/html/table03.html dom/xhtml/level2/html/HTMLTableElement24.xhtml dom/html/level2/html/HTMLTableElement19.html dom/xhtml/level2/html/HTMLTableElement28.xhtml dom/xhtml/level2/html/HTMLCollection08.xhtml dom/xhtml/level2/html/HTMLTableRowElement01.xhtml dom/xhtml/level2/html/HTMLTableElement30.xhtml accessibility/th-as-title-ui.html dom/xhtml/level2/html/HTMLCollection01.xhtml dom/html/level2/html/HTMLCollection08.html accessibility/table-with-rules.html dom/xhtml/level2/html/HTMLCollection06.xhtml dom/html/level2/html/HTMLCollection07.html dom/html/level2/html/HTMLCollection01.html Created attachment 217420 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 217406 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217406&action=review > Source/WebCore/html/HTMLTableElement.cpp:76 > + if (auto firstCaption = childrenOfType<HTMLTableCaptionElement>(*this).first()) > + return const_cast<HTMLTableCaptionElement*>(firstCaption); > + > + return nullptr; Can't we just do: return const_cast<HTMLTableCaptionElement*>(childrenOfType<HTMLTableCaptionElement>(*this).first()); ? Comment on attachment 217406 [details] Patch Attachment 217406 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/30058089 New failing tests: dom/html/level2/html/HTMLCollection06.html dom/xhtml/level2/html/HTMLTableElement21.xhtml dom/html/level2/html/HTMLTableElement28.html dom/html/level2/html/HTMLTableElement22.html dom/html/level2/html/HTMLTableRowElement01.html accessibility/table-one-cell.html dom/html/level2/html/HTMLTableElement24.html dom/html/level2/html/HTMLTableElement30.html dom/xhtml/level2/html/HTMLCollection07.xhtml dom/xhtml/level2/html/HTMLTableElement19.xhtml dom/html/level2/html/HTMLTableElement04.html dom/html/level2/html/HTMLTableElement06.html dom/html/level2/html/HTMLTableElement21.html dom/xhtml/level2/html/HTMLTableElement22.xhtml accessibility/table-detection.html dom/html/level2/html/table03.html dom/xhtml/level2/html/HTMLTableElement24.xhtml dom/html/level2/html/HTMLTableElement19.html dom/xhtml/level2/html/HTMLTableElement28.xhtml accessibility/table-nofirstbody.html dom/xhtml/level2/html/HTMLCollection08.xhtml dom/xhtml/level2/html/HTMLTableElement30.xhtml accessibility/th-as-title-ui.html dom/xhtml/level2/html/HTMLCollection01.xhtml dom/html/level2/html/HTMLCollection08.html accessibility/table-with-rules.html dom/xhtml/level2/html/HTMLCollection06.xhtml dom/html/level2/html/HTMLCollection07.html dom/html/level2/html/HTMLCollection01.html accessibility/non-data-table-cell-title-ui-element.html Created attachment 217425 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 217406 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217406&action=review >> Source/WebCore/html/HTMLTableElement.cpp:76 >> + if (auto firstCaption = childrenOfType<HTMLTableCaptionElement>(*this).first()) >> + return const_cast<HTMLTableCaptionElement*>(firstCaption); >> + >> + return nullptr; > > Can't we just do: > return const_cast<HTMLTableCaptionElement*>(childrenOfType<HTMLTableCaptionElement>(*this).first()); > ? Yes, right, there’s no need for the if statement. A null pointer can be casted. > Source/WebCore/html/HTMLTableElement.cpp:90 > + if (auto firstSection = childrenOfType<HTMLTableSectionElement>(*this).first()) > + return const_cast<HTMLTableSectionElement*>(firstSection); > + > + return nullptr; This one is wrong. It needs to only return a thead, not any HTMLTableSectionElement. So there needs to be a loop. We could still use childrenOfType, but we need a loop. > Source/WebCore/html/HTMLTableElement.cpp:110 > + if (auto firstSection = childrenOfType<HTMLTableSectionElement>(*this).first()) > + return const_cast<HTMLTableSectionElement*>(firstSection); > + > + return nullptr; This one is wrong. It needs to only return a tfoot, not any HTMLTableSectionElement. So there needs to be a loop. We could still use childrenOfType, but we need a loop. > Source/WebCore/html/HTMLTableElement.cpp:180 > - for (Node* child = lastChild(); child; child = child->previousSibling()) { > - if (child->hasTagName(tbodyTag)) > - return toHTMLTableSectionElement(child); > - } > - return 0; > + if (auto lastSection = childrenOfType<HTMLTableSectionElement>(*this).last()) > + return const_cast<HTMLTableSectionElement*>(lastSection); > + > + return nullptr; This one is wrong. It needs to only return a tbody, not any HTMLTableSectionElement. So there needs to be a loop. We could still use childrenOfType, but we need a loop. This patch can be fixed after solving an issue mentioned in https://bugs.webkit.org/show_bug.cgi?id=124571#c14 Pull request: https://github.com/WebKit/WebKit/pull/22269 Committed 272797@main (7cc92197012b): <https://commits.webkit.org/272797@main> Reviewed commits have been landed. Closing PR #22269 and removing active labels. |