[webkit-reviews] review denied: [Bug 40775] Table Cell Layering : [Attachment 60647] Table Cell Layering Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Fady Samuel <fsamuel at chromium.org>'s
request for review:
Bug 40775: Table Cell Layering
https://bugs.webkit.org/show_bug.cgi?id=40775

Attachment 60647: Table Cell Layering Patch
https://bugs.webkit.org/attachment.cgi?id=60647&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
We tend to avoid "get" in method names in the render tree, so instead of
getPrimaryCellAt, just make the method primaryCellAt. This is more consistent
with primaryCell() anyway.

Please revert your changes to RenderTableCell.cpp.  It looks like you just
messed with whitespace there without making any real changes.

RenderTableSection::comparelayers... function names are always intercaps, so
that should be compareLayers.  I still think you need to avoid using the term
"layer" though, since that's very confusing.  Just rename to
compareCellPositions.

I think the implementation of primaryCell is a little weird duplicated.  Maybe
you could at least compact it to:

return hasCells() ? cells[cells.size() - 1] : 0;

That's one line instead of three and doesn't look so bad used twice. :)

The RenderTableSection member, Vector<RenderTableCell*> m_Cells, does not
appear to be used unless I'm missing something.