Patch for when rowspan or colspan equals 0

Dear kfm-devel list:
        I've been working on a patch to solve the behaviour for when a a cell is
defined to have a rowspan or a colspan of 0. Attached is a patch which nearly
solves the problem. The colspan is solved but the rowspan, even though the
code is doing nearly the same as for colspan, of course adapted for rows; and
it's behaving like I intended, is not solving the problem yet. Let me explain
how's the process:
        ->If we find that the current column has a span of 0 we save it as "write this
cell in it's row whenever you reach the next column". That's it we save the
cell and we wait till we've changed the column, (say, from 0 to 1) and then we
mark the cell 0,0 as occupied and update the span properly (from 1 to 2 in
this case).
        ->For rows is the same story, we save the cell and the column number whenever
we find ourselves in the same column we take ownership of the cell and
increment the span by one.
        This seems to work perfectly for the colspan but for some reason I cannot
understand it just doesn't cut it for the rowspan after span > 2.
        That's why I thought I had already solved it 'couse I was testing
testcase.html. But when I incremented the number of rows (in testcase2.html)
the cell wasn't expanded properly. Right now I can't answer why so although
I'll keep looking I decided to let new eyes have a look to this.
  Something even weirder happens if you test case found in
http://bugsfiles.kde.org/attachment.cgi?id=8334 even though it behaves nearly
like the testcase2.html (in which is not expanded beyond rowspan>2) it has a
new behaviour: one of the cells is dropped! (this is quite a surprise and
*might* not be my fault since it seems that what I receive is that row as the
current one), so further investigation is needed, but we're almost there.

        Finally I'm concerned that since I need to change the colspan and rowspan of
the cell from 0 the the actual number we might change the behaviour when, say,
a new row/column is inserted... will it reset the span to 0 so that we can
recalculate it properly again or will it just live happily thinking that its
span must not be recalculated and hence leaving a row or a column not covered?
if so, should we add a flag so that we see "oh there's this special cell,
recalculate it's span not meter which span it has"?. I can't answer those
questions right now and I'd like to have some help with them.

PS:remember the idea of inserting the cells on a later time, once all the rows
and cols are inserted? can't be done we need to mark the cell as used before
someone else takes it.
PPS: the patch is a little bigger than it should since I added comments and
proper indentation to the code so that it can be more easily read plus
KDevelop erased trailing spaces.

--
Carlos

celda 0,0
celda 1,0	celda 1,1	celda 1,2
celda 2,1	celda 2,2	celda 2,3

celda 0,0
celda 1,0	celda 1,1	celda 1,2
celda 2,1	celda 2,2	celda 2,3
celda 3,1	celda 3,2	celda 3,3
celda 4,1	celda 4,2	celda 4,3

[tables.diff]

---

Index: html/html_tableimpl.cpp
===================================================================
--- html/html_tableimpl.cpp (revisión: 948444)
+++ html/html_tableimpl.cpp (copia de trabajo)
@@ -210,7 +210,7 @@
     lastSection = section;
     if ( index < 0 ) //append/last mode
         return 0;
-    
+
     long rows   = section->numRows();
     if ( index >= rows ) {
         section =  0;
@@ -428,7 +428,7 @@
 
     return cellChanged;
 }
-
+
 void HTMLTableElementImpl::parseAttribute(AttributeImpl *attr)
 {
     // ### to CSS!!
@@ -910,14 +910,14 @@
         // limit this to something not causing an overflow with short int
         if(rSpan < 0 || rSpan > 1024) rSpan = 1;
         if (renderer())
-            renderer()->updateFromElement();                    
+            renderer()->updateFromElement();
         break;
     case ATTR_COLSPAN:
         cSpan = attr->val() ? attr->val()->toInt() : 1;
         // limit this to something not causing an overflow with short int
         if(cSpan < 0 || cSpan > 1024) cSpan = 1;
         if (renderer())
-            renderer()->updateFromElement();        
+            renderer()->updateFromElement();
         break;
     case ATTR_NOWRAP:
         if (attr->val() != 0)
@@ -964,7 +964,7 @@
     {
     case ATTR_SPAN:
         _span = attr->val() ? attr->val()->toInt() : 1;
-        if (_span < 1) _span = 1;
+        if (_span < 0) _span = 1;
         break;
     case ATTR_WIDTH:
         if (!attr->value().isEmpty())
Index: rendering/render_table.cpp
===================================================================
--- rendering/render_table.cpp (revisión: 948444)
+++ rendering/render_table.cpp (copia de trabajo)
@@ -182,7 +182,7 @@
                     head = static_cast<RenderTableSection *>(child);
                 } else {
                     resetSectionPointerIfNotBefore(firstBody, beforeChild);
-                    if (!firstBody)
+                    if (!firstBody)
                         firstBody = static_cast<RenderTableSection*>(child);
                 }
             }
@@ -761,10 +761,10 @@
                 maxCols = sectionCols;
         }
     }
-    
+
     columns.resize(maxCols);
     columnPos.resize(maxCols+1);
-    
+
     needSectionRecalc = false;
     setNeedsLayout(true);
 }
@@ -1190,44 +1190,99 @@
  }
     }
 
+    //Postion the cell
+    //How: we take care of special case when colspan or rowspan equals 0
+    //after that position the cell, that is just to tell where is it and tell
+    //other cells where they can't be located (marking the cells as -1)
+    //later taking the span into account (and in other function) the cell is
+    //then painted (that's why we need to set the colspan and rowspan properly
+    //when any of them is cero
+
     // make sure we have enough rows
-    ensureRows( cRow + rSpan );
+    ensureRows( cRow + (rSpan)? rSpan : 1 );
 
     grid[cRow].rowRenderer = row;
 
     int col = cCol;
     // tell the cell where it is
     RenderTableCell *set = cell;
-    while ( cSpan ) {
- int currentSpan;
- if ( cCol >= nCols ) {
-    table()->appendColumn( cSpan );
-    currentSpan = cSpan;
- } else {
-    if ( cSpan < columns[cCol].span )
- table()->splitColumn( cCol, cSpan );
-    currentSpan = columns[cCol].span;
- }
- int r = 0;
- while ( r < rSpan ) {
-    if ( !cellAt( cRow + r, cCol ) ) {
-// qDebug("    adding cell at %d, %d",  cRow + r, cCol );
- cellAt( cRow + r, cCol ) = set;
-    }
-    r++;
- }
- cCol++;
- cSpan -= currentSpan;
- set = (RenderTableCell *)-1;
+    kDebug()<<"row"<<cRow<<"col"<<cCol;
+    //check wether we are in a column or in a row with span = 0
+    int effectiveCurrentCol = table()->effColToCol(cCol);
+    if( cellsWithColSpanZero.contains( effectiveCurrentCol ) ) {
+        while ( RenderTableCell* cell = cellsWithColSpanZero.take( effectiveCurrentCol ) ) {
+            int cellRow = cell->row();
+            if( !cellAt( cellRow, effectiveCurrentCol ) ) {
+                cellAt( cellRow, effectiveCurrentCol ) = (RenderTableCell *)-1;
+            }
+            cell->setColSpan( cell->colSpan() + 1 );
+            cellsWithColSpanZero.insertMulti(effectiveCurrentCol + 1, cell );
+        }
     }
+    if( cellsWithRowSpanZero.contains( cCol ) ) {
+        RenderTableCell* cell = cellsWithRowSpanZero.value( cCol );
+        if( !cellAt( cRow, cCol ) ) {
+            cellAt( cRow, cCol ) = (RenderTableCell *)-1;
+        }
+        cell->setRowSpan( cell->rowSpan() + 1 );
+        kDebug()<<"final rowspan"<<cell->rowSpan();
+    }
+
+    //Save the column if the its span is 0
+    if( cSpan == 0 ) {
+        kDebug()<<"save column:"<<cCol+1;
+        //mark it to be inserted in the next column
+        cellsWithColSpanZero.insertMulti( table()->effColToCol(cCol) + 1, cell );
+        if( cCol >= nCols ) {
+            table()->appendColumn( cCol - nCols );
+        }
+        if ( !cellAt( cRow, cCol ) ) {
+            cellAt( cRow, cCol ) = cell;
+        }
+        cell->setColSpan( 1 );
+    }
+
+    if( rSpan == 0 ) {
+        //mark it to be inserted every time we visit this column
+        cellsWithRowSpanZero.insert( cCol, cell );
+        if ( !cellAt( cRow, cCol ) ) {
+            cellAt( cRow, cCol ) = cell;
+        }
+        cell->setRowSpan( 1 );
+    }
+
+    if( rSpan ) {
+        while ( cSpan ) {
+            int currentSpan;
+            if ( cCol >= nCols ) {
+                table()->appendColumn( cSpan );
+                currentSpan = cSpan;
+            } else {
+                if ( cSpan < columns[cCol].span ) {
+                    table()->splitColumn( cCol, cSpan );
+                }
+                currentSpan = columns[cCol].span;
+            }
+            int r = 0;
+            while ( r < rSpan ) {
+                if ( !cellAt( cRow + r, cCol ) ) {
+    // qDebug("    adding cell at %d, %d",  cRow + r, cCol );
+                    cellAt( cRow + r, cCol ) = set;
+                }
+                r++;
+            }
+            cCol++;
+            cSpan -= currentSpan;
+            set = (RenderTableCell *)-1;
+        }
+    }
+
     if ( cell ) {
- cell->setRow( cRow );
- cell->setCol( table()->effColToCol( col ) );
+        cell->setRow( cRow );
+        cell->setCol( table()->effColToCol( col ) );
     }
 }
 
-
-
 void RenderTableSection::setCellWidths()
 {
 #ifdef DEBUG_LAYOUT
@@ -1781,7 +1836,7 @@
  if ( !cell || cell == (RenderTableCell *)-1 || nextrow && (*nextrow)[c] == cell )
     continue;
                 RenderObject* rowr = cell->parent();
-                int rtx = tx+rowr->xPos();
+                int rtx = tx+rowr->xPos();
                 int rty = ty+rowr->yPos();
 #ifdef TABLE_PRINT
  kDebug( 6040 ) << "painting cell " << r << "/" << c;
@@ -1826,17 +1881,17 @@
 int RenderTableSection::numColumns() const
 {
     int result = 0;
-    
+
     for (int r = 0; r < numRows(); ++r) {
         for (int c = result; c < table()->numEffCols(); ++c) {
             if (cellAt(r, c))
                 result = c;
         }
     }
-    
+
     return result + 1;
 }
-
+
 void RenderTableSection::recalcCells()
 {
     cCol = 0;
@@ -1854,6 +1909,7 @@
             for (RenderObject *cell = row->firstChild(); cell; cell = cell->nextSibling())
                 if (cell->isTableCell())
                     addCell( static_cast<RenderTableCell *>(cell), static_cast<RenderTableRow *>(row) );
+
  }
     }
     needCellRecalc = false;
@@ -2563,7 +2619,7 @@
     RenderTableCol* colElt = table()->colElement(col() + (rtl ? colSpan() - 1 : 0), &startColEdge, &endColEdge);
     if (colElt && (!rtl ? startColEdge : endColEdge)) {
         result = compareBorders(result, CollapsedBorderValue(&colElt->style()->borderLeft(), BCOL));
-        if (!result.exists())
+        if (!result.exists())
             return result;
         if (colElt->parent()->isTableCol() && (!rtl ? !colElt->previousSibling() : !colElt->nextSibling())) {
             result = compareBorders(result, CollapsedBorderValue(&colElt->parent()->style()->borderLeft(), BCOLGROUP));
Index: rendering/render_table.h
===================================================================
--- rendering/render_table.h (revisión: 948444)
+++ rendering/render_table.h (copia de trabajo)
@@ -273,6 +273,11 @@
 
     int numColumns() const;
 
+    //QMap< nextColumnToInsert, cell >
+    QMap< int, RenderTableCell* > cellsWithColSpanZero;
+    //QMap< column, cell >
+    QMap< int, RenderTableCell* > cellsWithRowSpanZero;
+
     signed short cRow;
     ushort cCol;
     bool needCellRecalc;

---