Summary: | Centralize and clean-up table column iteration | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Julien Chaffraix <jchaffraix> | ||||
Component: | Tables | Assignee: | Julien Chaffraix <jchaffraix> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | darin, eric, robert, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Julien Chaffraix
2012-05-21 15:08:09 PDT
Created attachment 143115 [details]
Proposed change. Clean-up lots of code and validate our current implementation.
Comment on attachment 143115 [details]
Proposed change. Clean-up lots of code and validate our current implementation.
LGTM. Should probably let the EWSes run.
Comment on attachment 143115 [details]
Proposed change. Clean-up lots of code and validate our current implementation.
Thanks Eric, clearing the cq until the EWS have had time to run!
:) Well, my bet was that cr-linux would be the slow one, and any of them would set cq- if they failed. :) But yes. Comment on attachment 143115 [details]
Proposed change. Clean-up lots of code and validate our current implementation.
EWS all green! \o/
Comment on attachment 143115 [details] Proposed change. Clean-up lots of code and validate our current implementation. Clearing flags on attachment: 143115 Committed r117988: <http://trac.webkit.org/changeset/117988> All reviewed patches have been landed. Closing bug. Comment on attachment 143115 [details] Proposed change. Clean-up lots of code and validate our current implementation. View in context: https://bugs.webkit.org/attachment.cgi?id=143115&action=review > Source/WebCore/rendering/RenderTableCol.cpp:123 > + if (RenderObject* firstChild = this->firstChild()) > + return toRenderTableCol(firstChild); What guarantees the first child of a RenderTableCol is always another RenderTableCol? I think this is missing an isTableCol check. Comment on attachment 143115 [details] Proposed change. Clean-up lots of code and validate our current implementation. View in context: https://bugs.webkit.org/attachment.cgi?id=143115&action=review >> Source/WebCore/rendering/RenderTableCol.cpp:123 >> + return toRenderTableCol(firstChild); > > What guarantees the first child of a RenderTableCol is always another RenderTableCol? I think this is missing an isTableCol check. I believe this is correct and doesn't need an extra check for the following reasons: * only column-groups can have children (see RenderTableCol::canHaveChildren). * for column-groups, we only allow columns to be our children per RenderTableCol::isChildAllowed (the logic is a bit outdated but that's the gist of it). |