[Webkit-unassigned] [Bug 40775] Table Cell Layering

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 21 12:26:07 PDT 2010


https://bugs.webkit.org/show_bug.cgi?id=40775


Dave Hyatt <hyatt at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #59037|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #8 from Dave Hyatt <hyatt at apple.com>  2010-06-21 12:26:07 PST ---
(From update of attachment 59037)
Some initial feedback:

(1) I don't think CellNode is necessary. You can just make CellStruct have a Vector<RenderTableCell, 1>.  I'd give CellStruct a cell() accessor to get the "primary" RenderTableCell for the code that is using that.
(2) m_layer is not a good name for a variable added to RenderTableCell, since it shadows the m_layer in one of RenderTableCell's base classes.  Let's not overload "layer" if we can help it, since that has a pretty clear meaning in WebCore rendering code.  Something more like m_overlapLevel would be better.
(3) I'm not convinced that the overlap level member variable is needed.  Adding 4 bytes to all RenderTableCells for this seems problematic (same with the extra boolean that was added).  Can't paint order be inferred from ordering in the CellStruct? Maybe the answer is no, but think about it.  I'd consider using hashes in the .cpp file though rather than bloating RenderTableCell for a rare case (e.g., like we do with column members in RenderBlock).
(4) I don't like the name renderCellAtTop.  Someone reading that is going to think "top" means "top of the table."  If we're talking about the notion of a principal/primary cell that occupies that slot, then maybe primaryCell is good enough.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list