Patch for when rowspan or colspan equals 0

[Carlos Licea]

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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: tables.diff
Type: text/x-patch
Size: 9996 bytes
Desc: not available
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20090409/beeebefc/attachment.bin>