Bug 12078 – Clean up RenderTable*

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

Going to attach a patch with some cleanup and reorganization.

Created attachment 12169 [details]
RenderTable* cleanup and reorg

No layout test regressions.

[Sam Weinig]

Comment on attachment 12169 [details]
RenderTable* cleanup and reorg

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;

Created attachment 12213 [details]
RenderTable* cleanup and reorg

Did as Sam asked, except for removing qdebug()s from the table layout classes, since it's outside the scope of this bug. Did another change that Sam suggested on IRC, which is the addition of recalcCellsIfNeeded().

[Sam Weinig]

Comment on attachment 12213 [details]
RenderTable* cleanup and reorg

A couple more nits

In FixedTableLayout.cpp:

put spaces around the infix '+'

m_width[cCol+i].setRawValue(w.type(), w.rawValue() * eSpan);

and here

if (m_width[cCol+i].isAuto() && w.type() != Auto) {
    m_width[cCol+i].setRawValue(w.type(), w.rawValue() * eSpan);

and here around the infix '-'

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


In RenderTableSection.cpp:

Convert to use max()

if (totalPercent > 100 * percentScaleFactor)
    totalPercent = 100 * percentScaleFactor;

Created attachment 12219 [details]
RenderTable* cleanup and reorg

Discussed it with Sam on IRC and made a few changes.

[Sam Weinig]

Comment on attachment 12219 [details]
RenderTable* cleanup and reorg

r=me!

[David Kilzer (:ddkilzer)]

Committed revision 18608.