Patch for when rowspan or colspan equals 0

[Carlos Licea]

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