[Webkit-unassigned] [Bug 12078] Clean up RenderTable*

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=12078


sam at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #12169|review?                     |review-
               Flag|                            |




------- Comment #2 from sam at webkit.org  2007-01-02 13:59 PDT -------
(From update of attachment 12169)
Looks great!! But r- for now.


In AutoTableLayout.cpp and FixedTableLayout.cpp:

Put spaces around infix operators.

+    m_table->columnPositions()[m_table->columnPositions().size()-1] = pos;

It should be alright to remove the remaining qDebug() calls from these files as
well.


In RenderTable.cpp:

Resize and fill can be done with one call to Vector::fill(value, newSize)

+    m_columnPos.resize(2);
+    m_columnPos.fill(0);
+    m_columns.resize(1);
+    m_columns.fill(ColumnStruct());

Put spaces around '=='

+    if (m_caption && m_caption->style()->captionSide()==CAPBOTTOM) {

Missing spaces around infix '-'

+    memmove(m_columns.data() + pos + 1, m_columns.data() + pos, (oldSize-pos)
* sizeof(ColumnStruct));

Use 'unsigned' instead of 'unsigned int'

+    for (unsigned int i = 0; i < m_columns.size(); i++)


In RenderTableCell.cpp:

in function updateFromElement() there is no reason to declare the variables
'oldRSpan' and 'oldCSpan' outside of the if-statement, since that is the only
place they are used.


In RenderTableCell.h:

No reason to be inconsistent here, choose either setFoo(int foo) or setFoo(int
f)

+    int col() const { return m_column; }
+    void setCol(int col) { m_column = col; }
+    int row() const { return m_row; }
+    void setRow(int r) { m_row = r; }


In RenderTableSection.cpp:

Convert to use max(a,b)

+            if (pos > m_rowPos[r + 1])
+                m_rowPos[r + 1] = pos;

same here

+            if (m_rowPos[r + 1] < bRowPos)
+                m_rowPos[r + 1] = bRowPos;

and here

+        if (m_rowPos[r + 1] < m_rowPos[r])
+            m_rowPos[r + 1] = m_rowPos[r];

Put spaces around infix '+'

+                cell->setPos(table()->columnPositions()[nEffCols] -
table()->columnPositions()[table()->colToEffCol(cell->col()+cell->colSpan())] +
hspacing, m_rowPos[rindx]);

Use '!foo' instead of 'foo == 0' (in a couple of identical places)

+    if (m_gridRows == 0 || totalCols == 0)

also here

+        if (endcol == 0 && tx + table()->columnPositions()[0] -
table()->outerBorderLeft() <= y + w + os)


In RenderTableSection.h:

* on the wrong side

+    virtual const char *renderName() const { return "RenderTableSection"; }

and here

virtual void addChild(RenderObject *child, RenderObject *beforeChild = 0);

and here

+    virtual void dump(TextStream *, DeprecatedString ind = "") const;


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