|
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;
|
|
Re: Patch for when rowspan or colspan equals 0
Dear list,
I've been working on my patch for implementing proper colspan and rowspan
behaviour.
Forget almost completely my previous mail, here's a new way of doing it, way
better than the old one, which has no known corner-cases, it even handles
properly when both colspan and rowspan equals 0 in the same cell, whereas the
last one did but I couldn't catch them till recently.
In essence the method now is:
->If we found a colspan = 0 we insert the cell and take ownership of all the
cells to the right that exist so far, update the colspan properly and insert
it in the QMap as such QMap<nextColWhenUpdateIsNeeded, cell>.
->Similarly for rowspan = 0, except that since the rows are sequential
whenever we find a row with such rowspan it has to be from that point on, it's
inserted similarly: QMap< nextRowWhenUpdateIsNeeded, cell>.
->On the next pass we check whether we're in an unhandled column for any of
the colspan=0 cells, if so, update properly (update its colspan and mark as
used the cell). We check the same for rowspans and store the column in which
no other cell should be placed.
->Process cell as usual, except if we're in a column in which no cell should
be inserted then advance to the next column.
The only corner case for both colspan and rowspan cells happens when it's a
new column in which we are dealing, that is we are at a position in which no
column has been inserted so far. Of course it is properly handled inserting a
new column.
So the algorithm seems to work quite well actually, except for one problem,
for rowspan > 2 for some reason it stops growing the cell even though we set
the rowspan properly! (I believe but my believings have been proven wrong in
the past ;).
So we're this time _really_ nearly there. I'm attaching you the bunch of test
cases I've covered, and passed, so far (the previous ones had some
misspellings and I didn't realize till after sending the previous mail so it
lead me to the wrong conclusion that the previous algorithm worked, now I
believe they're actually right), also the test in
http://bugsfiles.kde.org/attachment.cgi?id=8334 is passed too except for the
fact that rowspan doesn't seem to want to grow greater than 2.
I'll let you know if I could solve the rowspan > 2 problem, if so then the
patch wouldn't have any known problems, and would be waiting just for the
regression test to be commited.
--
Carlos
| cell 0,0 |
| cell 1,0 | cell 1,1 | cell 1,2 |
| cell 2,1 | cell 2,2 | cell 2,3 |
| cell 3,1 | cell 3,2 | cell 3,3 |
| cell 4,1 | cell 4,2 | cell 4,3 |
| cell 0,0 |
| cell 1,0 |
| cell 2,0 | cell 1,1 | cell 1,2 |
| cell 3,1 | cell 3,2 | cell 3,3 |
| cell 0,0 |
| cell 1,0 | cell 1,1 | cell 1,2 |
| cell 2,1 | cell 2,2 |
| cell 0,0 |
| cell 1,0 | cell 1,1 | cell 1,2 |
| cell 2,1 |
| cell 3,1 |
| cell 0,0 | cell 0,1 | cell 0,2 |
| cell 1,0 | cell 1,1 |
| cell 2,0 |
| cell 0,0 | cell 0,1 |
| cell 1,0 | cell 1,1 | cell 1,2 | cell 1,3 |
| cell 2,0 |
| cell 3,0 | cell 3,1 | cell 3,2 |
| cell 4,0 |
[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,115 @@
}
}
+ //What: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;
+ kDebug()<<"cSpan"<<cSpan<<"rSpan"<<rSpan;
+ //check wether we are in a column or in a row with span = 0
+ int actualCurrentCol = table()->effColToCol(cCol);
+ if( cellsWithColSpanZero.contains( actualCurrentCol ) ) {
+ while ( RenderTableCell* cell = cellsWithColSpanZero.take( actualCurrentCol ) ) {
+ int cellRow = cell->row();
+ if( !cellAt( cellRow, actualCurrentCol ) ) {
+ cellAt( cellRow, actualCurrentCol ) = (RenderTableCell *)-1;
+ }
+ cell->setColSpan( cell->colSpan() + 1 );
+ cellsWithColSpanZero.insertMulti(actualCurrentCol + 1, cell );
+ }
}
+ QList< int > columnsToAvoid;
+ if( cellsWithRowSpanZero.contains( cRow ) ) {
+ while( RenderTableCell* cell = cellsWithRowSpanZero.take( cRow ) ) {
+ int cellCol = cell->col();
+ if( !cellAt( cRow, cellCol ) ) {
+ cellAt( cRow, cellCol ) = (RenderTableCell *)-1;
+ }
+ cell->setRowSpan( cell->rowSpan() + 1 );
+ columnsToAvoid << cellCol;
+ cellsWithRowSpanZero.insertMulti( cRow + 1, cell );
+ kDebug()<<"final rowspan"<<cell->rowSpan();
+ }
+ }
+
+ //Save the column if the its span is 0
+ if( cSpan == 0 ) {
+ //mark it to be inserted in the next column
+ if( cCol >= nCols ) {
+ table()->appendColumn( cCol - nCols );
+ nCols = columns.size();
+ }
+ int finalSpan = 0;
+ for( int i = cCol; i < nCols; ++i) {
+ if ( !cellAt( cRow, cCol ) ) {
+ cellAt( cRow, cCol ) = cell;
+ }
+ ++finalSpan;
+ }
+ cellsWithColSpanZero.insertMulti( table()->effColToCol(cCol) + finalSpan, cell );
+ cell->setColSpan( finalSpan );
+ }
+
+ if( rSpan == 0 ) {
+ if( cCol >= nCols ) {
+ table()->appendColumn( cCol - nCols );
+ nCols = columns.size();
+ }
+ //mark is as inserted in next row
+ cellsWithRowSpanZero.insertMulti( cRow + 1, cell );
+ if ( !cellAt( cRow, cCol ) ) {
+ cellAt( cRow, cCol ) = cell;
+ }
+ cell->setRowSpan( 1 );
+ kDebug()<<"Adding rowspan0";
+ }
+
+ 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;
+ if( columnsToAvoid.contains(cCol) ) ++cCol;
+ 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 +1852,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 +1897,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 +1925,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 +2635,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;
|
|
Re: Patch for when rowspan or colspan equals 0
Le samedi 4 avril 2009, Carlos Licea a écrit :
> Dear list,
> I've been working on my patch for implementing proper colspan and rowspan
> behaviour.
> Forget almost completely my previous mail, here's a new way of doing it,
> way better than the old one, which has no known corner-cases, it even
> handles properly when both colspan and rowspan equals 0 in the same cell,
> whereas the last one did but I couldn't catch them till recently.
Hi Carlos,
nice work so far!
> In essence the method now is:
> ->If we found a colspan = 0 we insert the cell and take ownership of all
> the cells to the right that exist so far, update the colspan properly and
> insert it in the QMap as such QMap<nextColWhenUpdateIsNeeded, cell>.
> ->Similarly for rowspan = 0, except that since the rows are sequential
> whenever we find a row with such rowspan it has to be from that point on,
> it's inserted similarly: QMap< nextRowWhenUpdateIsNeeded, cell>.
> ->On the next pass we check whether we're in an unhandled column for any
> of the colspan=0 cells, if so, update properly (update its colspan and mark
> as used the cell). We check the same for rowspans and store the column in
> which no other cell should be placed.
> ->Process cell as usual, except if we're in a column in which no cell
> should be inserted then advance to the next column.
> The only corner case for both colspan and rowspan cells happens when it's a
> new column in which we are dealing, that is we are at a position in which
> no column has been inserted so far. Of course it is properly handled
> inserting a new column.
>
OK.
> So the algorithm seems to work quite well actually, except for one problem,
> for rowspan > 2 for some reason it stops growing the cell even though we
> set the rowspan properly! (I believe but my believings have been proven
> wrong in the past ;).
errm I'm getting Qt assert failures (index out of range) in QVector's []
operator with all your rowspan testcases, so some logic is indeed wrong.
it sounds like this could all stem from:
+ ensureRows( cRow + (rSpan)? rSpan : 1 );
? surely, you meant:
+ ensureRows( cRow + (rSpan? rSpan : 1) );
hmm, no, doesn't seem to be enough to fix it.
anyway, some comments on the patch so far:
- the html/html_tableimpl.cpp bit about changing the lowest allowed value for
the span attribute of the col element's :
@@ -964,7 +964,7 @@
{
case ATTR_SPAN:
_span = attr->val() ? attr->val()->toInt() : 1;
- if (_span < 1) _span = 1;
+ if (_span < 0) _span = 1;
looks wrong actually... the HTMl 4.01 specification mandates that this value
be > 0
- could you setup a boolean flag for when no zero rowspan/colspan has been
encountered yet, and then skip entirely the map lookup logic and effColToCol
calculation as an optimization?
- you should clear your two maps on recalculation (up in recalcCells()
probably)
- this does not cover exactly the 4.01 specification for colspan,
where the spanning of colspan=0 cells should be limited to the current
colgroup.
Figuring the extent of the current colgroup is not really straightforward
however, as colgroup (and col) are sort of descriptive tags that have no real
rendered counterpart.
Yet we have (zero dimensions) render objects for them in the render tree.
e.g:
<table>
<colgroup span=2></colgroup>
<colgroup span=3></colgroup>
<colgroup>
<col span=2>
<col>
</colgroup>
</table>
defining 3 colgroups spanning 8 logical cells in total,
will give you a render tree as such:
RenderTable: <table>
RenderTableCol: <colgroup>
RenderTableCol: <colgroup>
RenderTableCol: <colgroup>
RenderTableCol:<col>
RenderTableCol:<col>
See for instance RenderTable::colElement for a method that's walking this
structure.
A full table matching that colgroup definition would be for instance:
<table>
<colgroup span=2></colgroup>
<colgroup span=3></colgroup>
<colgroup>
<col span=2>
<col>
</colgroup>
<tr>
<td colspan=2>1st colgroup</td>
<td>2nd</td><td>col</td><td>group</td>
<td colspan=3>3rd colgroup</td>
</tr>
</table>
> So we're this time _really_ nearly there. I'm attaching you the bunch of
> test cases I've covered, and passed, so far (the previous ones had some
> misspellings and I didn't realize till after sending the previous mail so
> it lead me to the wrong conclusion that the previous algorithm worked, now
> I believe they're actually right), also the test in
> http://bugsfiles.kde.org/attachment.cgi?id=8334 is passed too except for
> the fact that rowspan doesn't seem to want to grow greater than 2.
> I'll let you know if I could solve the rowspan > 2 problem, if so then the
> patch wouldn't have any known problems, and would be waiting just for the
> regression test to be commited.
Thanks,
Germain
|
|
Re: Patch for when rowspan or colspan equals 0
for reference:
http://www.w3.org/TR/html401/struct/tables.html#h-11.2.6#
#rowspan = number [CN]
#This attribute specifies the number of rows spanned by the current cell. The
#default value of this attribute is one ("1"). The value zero ("0") means
#that the cell spans all rows from the current row to the last row of the
#table section (THEAD, TBODY, or TFOOT) in which the cell is defined.
#
#colspan = number [CN]
#This attribute specifies the number of columns spanned by the current cell.
#The default value of this attribute is one ("1"). The value zero ("0") means
#that the cell spans all columns from the current column to the last column
#of the column group (COLGROUP) in which the cell is defined.
#
this does not explicitely cover the case where no colgroup is defined, but it
seems safe to assume, given the behaviour for rowspan, that the cell should
span all the table section then.
|
|
Re: Patch for when rowspan or colspan equals 0
On Lunes 06 Abril 2009 21:55:33 Germain Garand escribió:
> Le samedi 4 avril 2009, Carlos Licea a écrit :
> > Dear list,
> > I've been working on my patch for implementing proper colspan and
> > rowspan behaviour.
> > Forget almost completely my previous mail, here's a new way of doing it,
> > way better than the old one, which has no known corner-cases, it even
> > handles properly when both colspan and rowspan equals 0 in the same cell,
> > whereas the last one did but I couldn't catch them till recently.
>
> Hi Carlos,
> nice work so far!
>
Thanks, I believe is shaping up nicely, we still have some work to do, as you
uncovered, though.
> > So the algorithm seems to work quite well actually, except for one
> > problem, for rowspan > 2 for some reason it stops growing the cell even
> > though we set the rowspan properly! (I believe but my believings have
> > been proven wrong in the past ;).
>
> errm I'm getting Qt assert failures (index out of range) in QVector's []
> operator with all your rowspan testcases, so some logic is indeed wrong.
>
You were right at IRC, I missed the case when you already found the cell which
has a colspan = 0 and you want to insert it in a new column, if I'm correct it
just happens when the colspan=0 is in the first row.
> it sounds like this could all stem from:
>
> + ensureRows( cRow + (rSpan)? rSpan : 1 );
>
> ? surely, you meant:
>
> + ensureRows( cRow + (rSpan? rSpan : 1) );
>
Changed, though... is there a real difference? I didn't get any compiling
errors.
> - the html/html_tableimpl.cpp bit about changing the lowest allowed value
> for the span attribute of the col element's :
>
> @@ -964,7 +964,7 @@
> {
> case ATTR_SPAN:
> _span = attr->val() ? attr->val()->toInt() : 1;
> - if (_span < 1) _span = 1;
> + if (_span < 0) _span = 1;
>
> looks wrong actually... the HTMl 4.01 specification mandates that this
> value be > 0
>
Changed it back.
> - could you setup a boolean flag for when no zero rowspan/colspan has been
> encountered yet, and then skip entirely the map lookup logic and
> effColToCol calculation as an optimization?
>
Done.
> - you should clear your two maps on recalculation (up in recalcCells()
> probably)
>
Done. Though for it to work properly we must make sure that the previous cells
aren't somehow used, otherwise we would have a span different than 0 and we
would have a 'normal' cell instead of a special case cell.
> - this does not cover exactly the 4.01 specification for colspan,
> where the spanning of colspan=0 cells should be limited to the current
> colgroup.
> Figuring the extent of the current colgroup is not really straightforward
> however, as colgroup (and col) are sort of descriptive tags that have no
> real rendered counterpart.
>
> Yet we have (zero dimensions) render objects for them in the render tree.
>
> e.g:
> <table>
> <colgroup span=2></colgroup>
> <colgroup span=3></colgroup>
> <colgroup>
> <col span=2>
> <col>
> </colgroup>
> </table>
>
I need to check that use case...
> defining 3 colgroups spanning 8 logical cells in total,
> will give you a render tree as such:
>
> RenderTable: <table>
> RenderTableCol: <colgroup>
> RenderTableCol: <colgroup>
> RenderTableCol: <colgroup>
> RenderTableCol:<col>
> RenderTableCol:<col>
>
>
> See for instance RenderTable::colElement for a method that's walking this
> structure.
I'll have a look.
>
> A full table matching that colgroup definition would be for instance:
> <table>
> <colgroup span=2></colgroup>
> <colgroup span=3></colgroup>
> <colgroup>
> <col span=2>
> <col>
> </colgroup>
>
> <tr>
> <td colspan=2>1st colgroup</td>
> <td>2nd</td><td>col</td><td>group</td>
> <td colspan=3>3rd colgroup</td>
> </tr>
>
> </table>
>
So this is a real table which I can test against?
--
Carlos
[tables.diff]
Index: html/html_tableimpl.cpp
===================================================================
--- html/html_tableimpl.cpp (revisión: 950904)
+++ 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)
Index: rendering/render_table.cpp
===================================================================
--- rendering/render_table.cpp (revisión: 950904)
+++ 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);
}
@@ -1028,6 +1028,7 @@
RenderTableSection::RenderTableSection(DOM::NodeImpl* node)
: RenderBox(node)
+ , containsSpansZero(false)
{
// init RenderObject attributes
setInline(false); // our object is not Inline
@@ -1190,44 +1191,130 @@
}
}
+ //What: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;
+// kDebug()<<"cSpan"<<cSpan<<"rSpan"<<rSpan;
+
+ //check wether we are in a column or in a row with span = 0
+ QList< int > columnsToAvoid;
+ if( containsSpansZero ) {
+ int actualCurrentCol = table()->effColToCol(cCol);
+ if( cellsWithColSpanZero.contains( actualCurrentCol ) ) {
+ //No need to check if we have enough column, we already found the first cell
+ //(when colspan="0", and as such, we've already inserted it
+ while ( RenderTableCell* cell = cellsWithColSpanZero.take( actualCurrentCol ) ) {
+ int cellRow = cell->row();
+ if( !cellAt( cellRow, actualCurrentCol ) ) {
+ cellAt( cellRow, actualCurrentCol ) = (RenderTableCell *)-1;
+ }
+ cell->setColSpan( cell->colSpan() + 1 );
+ cellsWithColSpanZero.insertMulti(actualCurrentCol + 1, cell );
+ }
+ }
+
+ if( cellsWithRowSpanZero.contains( cRow ) ) {
+ //make sure we have enough columns
+ if( cCol >= nCols ) {
+ table()->appendColumn( cCol - nCols );
+ nCols = columns.size();
+ }
+ while( RenderTableCell* cell = cellsWithRowSpanZero.take( cRow ) ) {
+ int cellCol = cell->col();
+ if( !cellAt( cRow, cellCol ) ) {
+ cellAt( cRow, cellCol ) = cell;
+ }
+ cell->setRowSpan( cell->rowSpan() + 1 );
+ columnsToAvoid << cellCol;
+ cellsWithRowSpanZero.insertMulti( cRow + 1, cell );
+// kDebug()<<"final rowspan"<<cell->rowSpan();
+ }
+ }
}
+
+ //Save the column if the its span is 0
+ if( cSpan == 0 ) {
+ //mark it to be inserted in the next column
+ if( cCol >= nCols ) {
+ table()->appendColumn( cCol - nCols );
+ nCols = columns.size();
+ }
+ int finalSpan = 0;
+ for( int i = cCol; i < nCols; ++i) {
+ if ( !cellAt( cRow, cCol ) ) {
+ cellAt( cRow, cCol ) = cell;
+ }
+ ++finalSpan;
+ }
+ cellsWithColSpanZero.insertMulti( table()->effColToCol(cCol) + finalSpan, cell );
+ cell->setColSpan( finalSpan );
+ containsSpansZero = true;
+ }
+
+ if( rSpan == 0 ) {
+ if( cCol >= nCols ) {
+ table()->appendColumn( cCol - nCols );
+ nCols = columns.size();
+ }
+ //mark is as inserted in next row
+ cellsWithRowSpanZero.insertMulti( cRow + 1, cell );
+ if ( !cellAt( cRow, cCol ) ) {
+ cellAt( cRow, cCol ) = cell;
+ }
+ cell->setRowSpan( 1 );
+ containsSpansZero = true;
+ }
+
+ 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;
+
+ if( columnsToAvoid.contains(cCol) )
+ ++cCol;
+
+ 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 +1868,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,23 +1913,25 @@
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;
cRow = -1;
clearGrid();
grid.resize( 0 );
+ cellsWithColSpanZero.clear();
+ cellsWithRowSpanZero.clear();
for (RenderObject *row = firstChild(); row; row = row->nextSibling()) {
if (row->isTableRow()) {
@@ -1854,6 +1943,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 +2653,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: 950904)
+++ rendering/render_table.h (copia de trabajo)
@@ -273,6 +273,12 @@
int numColumns() const;
+ //QMap< nextColumnToInsert, cell >
+ QMap< int, RenderTableCell* > cellsWithColSpanZero;
+ //QMap< nextRowToInsert, cell >
+ QMap< int, RenderTableCell* > cellsWithRowSpanZero;
+ bool containsSpansZero;
+
signed short cRow;
ushort cCol;
bool needCellRecalc;
|
|
Re: Patch for when rowspan or colspan equals 0
On Jueves 09 Abril 2009 15:42:23 Carlos Licea escribió:
> On Lunes 06 Abril 2009 21:55:33 Germain Garand escribió:
> > Le samedi 4 avril 2009, Carlos Licea a écrit :
> > > Dear list,
> > > I've been working on my patch for implementing proper colspan and
> > > rowspan behaviour.
> > > Forget almost completely my previous mail, here's a new way of doing
> > > it, way better than the old one, which has no known corner-cases, it
> > > even handles properly when both colspan and rowspan equals 0 in the
> > > same cell, whereas the last one did but I couldn't catch them till
> > > recently.
> >
> > Hi Carlos,
> > nice work so far!
>
> Thanks, I believe is shaping up nicely, we still have some work to do, as
> you uncovered, though.
>
> > > So the algorithm seems to work quite well actually, except for one
> > > problem, for rowspan > 2 for some reason it stops growing the cell even
> > > though we set the rowspan properly! (I believe but my believings have
> > > been proven wrong in the past ;).
> >
> > errm I'm getting Qt assert failures (index out of range) in QVector's []
> > operator with all your rowspan testcases, so some logic is indeed wrong.
>
> You were right at IRC, I missed the case when you already found the cell
> which has a colspan = 0 and you want to insert it in a new column, if I'm
> correct it just happens when the colspan=0 is in the first row.
>
> > it sounds like this could all stem from:
> >
> > + ensureRows( cRow + (rSpan)? rSpan : 1 );
> >
> > ? surely, you meant:
> >
> > + ensureRows( cRow + (rSpan? rSpan : 1) );
>
> Changed, though... is there a real difference? I didn't get any compiling
> errors.
>
> > - the html/html_tableimpl.cpp bit about changing the lowest allowed value
> > for the span attribute of the col element's :
> >
> > @@ -964,7 +964,7 @@
> > {
> > case ATTR_SPAN:
> > _span = attr->val() ? attr->val()->toInt() : 1;
> > - if (_span < 1) _span = 1;
> > + if (_span < 0) _span = 1;
> >
> > looks wrong actually... the HTMl 4.01 specification mandates that this
> > value be > 0
>
> Changed it back.
>
> > - could you setup a boolean flag for when no zero rowspan/colspan has
> > been encountered yet, and then skip entirely the map lookup logic and
> > effColToCol calculation as an optimization?
>
> Done.
>
> > - you should clear your two maps on recalculation (up in recalcCells()
> > probably)
>
> Done. Though for it to work properly we must make sure that the previous
> cells aren't somehow used, otherwise we would have a span different than 0
> and we would have a 'normal' cell instead of a special case cell.
>
> > - this does not cover exactly the 4.01 specification for colspan,
> > where the spanning of colspan=0 cells should be limited to the current
> > colgroup.
> > Figuring the extent of the current colgroup is not really straightforward
> > however, as colgroup (and col) are sort of descriptive tags that have no
> > real rendered counterpart.
> >
> > Yet we have (zero dimensions) render objects for them in the render tree.
> >
> > e.g:
> > <table>
> > <colgroup span=2></colgroup>
> > <colgroup span=3></colgroup>
> > <colgroup>
> > <col span=2>
> > <col>
> > </colgroup>
> > </table>
>
> I need to check that use case...
>
> > defining 3 colgroups spanning 8 logical cells in total,
> > will give you a render tree as such:
> >
> > RenderTable: <table>
> > RenderTableCol: <colgroup>
> > RenderTableCol: <colgroup>
> > RenderTableCol: <colgroup>
> > RenderTableCol:<col>
> > RenderTableCol:<col>
> >
> >
> > See for instance RenderTable::colElement for a method that's walking this
> > structure.
>
> I'll have a look.
>
> > A full table matching that colgroup definition would be for instance:
> > <table>
> > <colgroup span=2></colgroup>
> > <colgroup span=3></colgroup>
> > <colgroup>
> > <col span=2>
> > <col>
> > </colgroup>
> >
> > <tr>
> > <td colspan=2>1st colgroup</td>
> > <td>2nd</td><td>col</td><td>group</td>
> > <td colspan=3>3rd colgroup</td>
> > </tr>
> >
> > </table>
>
> So this is a real table which I can test against?
> --
> Carlos
I apologize for that message, was intended to arrive a week ago, for some
reason I couldn't send it (nonsenses of hotmail). Please just ignore it
completely. Also, expect to hear from me again about this =).
--
Carlos
|
|
Re: Patch for when rowspan or colspan equals 0
Some parts of this message have been removed.
Learn more about Nabble's security policy.
Hi list,
I had forgotten to submit this patch.
Now i believe it should be safe enough to commit.
Also I'm tired of this code, so I'll stop my work for now and start implementing the DOM Level 3 XPath specification.
Carlos
[tables.diff]
Index: html/html_tableimpl.cpp
===================================================================
--- html/html_tableimpl.cpp (revisión: 954616)
+++ 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)
Index: rendering/render_table.cpp
===================================================================
--- rendering/render_table.cpp (revisión: 954616)
+++ rendering/render_table.cpp (copia de trabajo)
@@ -9,6 +9,7 @@
* (C) 2003 Apple Computer, Inc.
* (C) 2005 Allan Sandfeld Jensen (kde@...)
* (C) 2008 Germain Garand (germain@...)
+ * (C) 2009 Carlos Licea (carlos.licea@...
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -182,7 +183,7 @@
head = static_cast<RenderTableSection *>(child);
} else {
resetSectionPointerIfNotBefore(firstBody, beforeChild);
- if (!firstBody)
+ if (!firstBody)
firstBody = static_cast<RenderTableSection*>(child);
}
}
@@ -761,10 +762,10 @@
maxCols = sectionCols;
}
}
-
+
columns.resize(maxCols);
columnPos.resize(maxCols+1);
-
+
needSectionRecalc = false;
setNeedsLayout(true);
}
@@ -1028,6 +1029,7 @@
RenderTableSection::RenderTableSection(DOM::NodeImpl* node)
: RenderBox(node)
+ , containsSpansZero(false)
{
// init RenderObject attributes
setInline(false); // our object is not Inline
@@ -1156,7 +1158,7 @@
// <TR><TD colspan="2">5
// </TABLE>
while ( cCol < nCols && cellAt( cRow, cCol ) )
- cCol++;
+ ++cCol;
// qDebug("adding cell at %d/%d span=(%d/%d)", cRow, cCol, rSpan, cSpan );
@@ -1190,44 +1192,158 @@
}
}
+ //What: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;
+ kDebug()<<"row"<<cRow<<"col"<<cCol;
+// kDebug()<<"cSpan"<<cSpan<<"rSpan"<<rSpan;
+
+ //check wether we are in a column or in a row with span = 0
+ QList< int > columnsToAvoid;
+ if( containsSpansZero ) {
+ //Update any column which its last span update was in a previous column
+ int lowestCol = cellsWithColSpanZero.lowerBound( 0 ).key();
+ if( lowestCol < cCol ) {
+ //add the columns for this cell
+ if ( cCol >= nCols ) {
+ table()->appendColumn( cSpan );
+ nCols = columns.size();
+ }
+ //check and update all the cells that are updated to a previous column
+ while( lowestCol < nCols ) {
+ while( RenderTableCell* cell = cellsWithColSpanZero.take( lowestCol ) ) {
+ const int cellRow = cell->row();
+ const int cellRowSpan = cell->rowSpan();
+ int finalSpan = cell->colSpan();
+ for( int i = lowestCol; i < nCols; ++i ) {
+ if( !cellAt( cellRow, i ) ) {
+ cellAt( cellRow, i ) = (RenderTableCell *)-1;
+ }
+ ++finalSpan;
+ }
+ cell->setColSpan( finalSpan );
+ cellsWithColSpanZero.insertMulti( nCols, cell );
+ }
+ lowestCol = cellsWithColSpanZero.lowerBound( 0 ).key();
+ }
+ }
+ if( cellsWithRowSpanZero.contains( cRow ) ) {
+ //No need to check if we have enough columns, we already found the first cell
+ //when rowspan="0", and as such, we've already inserted it
+ while( RenderTableCell* cell = cellsWithRowSpanZero.take( cRow ) ) {
+ const int cellCol = cell->col();
+ const int finalCol = cellCol + cell->colSpan() - 1;
+ RenderTableCell* set = cell;
+ for( int i = cellCol; i <= finalCol; ++i ) {
+ if( !cellAt( cRow, i ) ) {
+ cellAt( cRow, i ) = set;
+ }
+ set = (RenderTableCell *)-1;
+ columnsToAvoid << i;
+ }
+ cell->setRowSpan( cell->rowSpan() + 1 );
+ //mark it to be inserted in next row
+ cellsWithRowSpanZero.insertMulti( cRow + 1, cell );
+ }
+ }
+ }
+
+ //Save the column if the its span is 0
+ if( !cSpan ) {
+ //Check if we only span in the current colgroup
+ if( RenderTableCol* colgroup = table()->colElement( cCol ) ) {
+ //Calculate the correct span and then handle the cell normally
+
+ int firstColumnOfColgroup = cCol;
+ while( --firstColumnOfColgroup >= 0 && colgroup == table()->colElement(firstColumnOfColgroup) ) ;
+ ++firstColumnOfColgroup;
+
+ int alreadyUsedSpan = 0;
+ RenderTableCell* colgroupCell = cellAt( cRow, firstColumnOfColgroup + alreadyUsedSpan );
+ while( firstColumnOfColgroup + alreadyUsedSpan < cCol ) {
+ alreadyUsedSpan += colgroupCell->colSpan();
+ colgroupCell = cellAt( cRow, firstColumnOfColgroup + alreadyUsedSpan );
+ }
+
+ const int finalSpan = colgroup->span() - alreadyUsedSpan;
+ cell->setColSpan( finalSpan );
+
+ //We know exactly the cSpan so we can handle the cell as a normal cell
+ //unless, of course, the rowspan is also 0
+ cSpan = finalSpan;
+ }
+ //or if we span in the whole table and mark it as inserted till
+ //this column and updated whenever another column is inserted
+ else {
+ //We define the proper span and let the other code handle it
+ const int finalSpan = nCols - cCol;
+ cSpan = finalSpan;
+
+ cell->setColSpan( finalSpan );
+
+ cellsWithColSpanZero.insertMulti( cCol + finalSpan - 1, cell );
+ containsSpansZero = true;
+ }
+ }
+
+ if( !rSpan ) {
+ //For now we span 1
+ rSpan = 1;
+ cell->setRowSpan( 1 );
+
+ //mark it to be inserted in next row
+ cellsWithRowSpanZero.insertMulti( cRow + 1, cell );
+ containsSpansZero = true;
+ }
+
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;
+ int currentSpan;
+ if ( cCol >= nCols ) {
+ table()->appendColumn( cSpan );
+ currentSpan = cSpan;
+ } else {
+ if ( cSpan < columns[cCol].span ) {
+ table()->splitColumn( cCol, cSpan );
+ }
+ currentSpan = columns[cCol].span;
+ }
+
+ while( columnsToAvoid.contains(cCol) )
+ ++cCol;
+
+ 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 +1897,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,23 +1942,25 @@
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;
cRow = -1;
clearGrid();
grid.resize( 0 );
+ cellsWithColSpanZero.clear();
+ cellsWithRowSpanZero.clear();
for (RenderObject *row = firstChild(); row; row = row->nextSibling()) {
if (row->isTableRow()) {
@@ -1854,6 +1972,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 +2682,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: 954616)
+++ rendering/render_table.h (copia de trabajo)
@@ -273,6 +273,12 @@
int numColumns() const;
+ //QMap< nextColumnToInsert, cell >
+ QMap< int, RenderTableCell* > cellsWithColSpanZero;
+ //QMap< nextRowToInsert, cell >
+ QMap< int, RenderTableCell* > cellsWithRowSpanZero;
+ bool containsSpansZero;
+
signed short cRow;
ushort cCol;
bool needCellRecalc;
|
|
Re: Patch for when rowspan or colspan equals 0
Le dimanche 26 avril 2009, Carlos Licea a écrit :
> Hi list,
> I had forgotten to submit this patch.
> Now i believe it should be safe enough to commit.
> Also I'm tired of this code, so I'll stop my work for now and start
> implementing the DOM Level 3 XPath specification.
Hi Carlos,
this looks excellent to me.
It passes all testcases I can throw at it, and keeps the regression test suite
happy.
If you recall, I had concerns about whether this could be applied in all
rendering modes, seeing how Opera does make a distinction - but I now think
we may try this in all modes - seeing the old HTML2 meaning of
colspan/rowspan zero seems hardly ever used anywhere.
All I'm left to comment about is typos, otherwise I think its ripe for
commit.. ;)
+ * (C) 2009 Carlos Licea ( carlos.licea@...
missing parenthesis at end of line
+ //What:Postion the cell
missing 'i'
+ //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)
Up to here, all good. But the three lines below don't bring much clarity I
think... maybe replace them with a link to the relevant section of the HTML
4.01 specification?
+ //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
cero -> zero
Thank you for implementing a tricky bit of HTML 4, that no other engine ever
managed to implement correctly yet.
Greetings,
Germain
P.S: also please remove/comment the debug statements before commiting, as they
are a bit noisy.
|
|
Re: Patch for when rowspan or colspan equals 0
Some parts of this message have been removed.
Learn more about Nabble's security policy.
On Lunes 04 Mayo 2009 18:39:39 Germain Garand escribió:
> Le dimanche 26 avril 2009, Carlos Licea a écrit :
> > Hi list,
> > I had forgotten to submit this patch.
> > Now i believe it should be safe enough to commit.
> > Also I'm tired of this code, so I'll stop my work for now and start
> > implementing the DOM Level 3 XPath specification.
>
> Hi Carlos,
> this looks excellent to me.
>
Excellent!
> It passes all testcases I can throw at it, and keeps the regression test
> suite happy.
So happy to hear, I had to really scratch my head a while.
> If you recall, I had concerns about whether this could be applied in all
> rendering modes, seeing how Opera does make a distinction - but I now think
> we may try this in all modes - seeing the old HTML2 meaning of
> colspan/rowspan zero seems hardly ever used anywhere.
So I just commit as is, without checking the mode, right?
> All I'm left to comment about is typos, otherwise I think its ripe for
> commit.. ;)
Oops, even one is an Spanish word, hehe.
> + //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)
>
> Up to here, all good. But the three lines below don't bring much clarity I
> think... maybe replace them with a link to the relevant section of the HTML
> 4.01 specification?
What I meant is that the span is really used and that the cellAt() logic is only for positioning and not taken into account when painting, I'll try to clarify it and also link to the HTML spec.
> + //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
>
> cero -> zero
>
> Thank you for implementing a tricky bit of HTML 4, that no other engine
> ever managed to implement correctly yet.
No problem!
> Greetings,
> Germain
>
> P.S: also please remove/comment the debug statements before commiting, as
> they are a bit noisy.
Done, commented them. Don't worry I usually remove them before committing.
Also, I'll, finally, close the bug 41063.
Carlos
|
