Re: [Libreoffice] [Review] set all borders correctly in SvxRTFParser::ReadBorderAttr instead of only one

Hi Kendy,

On Fri, 2011-05-27 at 07:43 +0200, Jan Holesovsky wrote:
> > here is a short patch that sets all specified borders during parsing
> > instead of only one border. The problem was that nBorderTyp was set
> > for every border with the correct value but SetBorderLine was only
> > called once after the do while loop. So now every time nBorderType
> > will be overriden, I call SetBorderLine.
> >
> > If this patch is ok I think we should add it to the 3-4 branch.
>
> Cedric already approved the patch, 

Well, I haven't actually pushed the patch (had to check as I wasn't
sure). I was about to do it but I found some remaining problems with
some border properties not being copied properly (width for example).

> but I am wondering - before, the
> nBorderTyp was set only when bTableDef was true; after your patch, it is
> set regardless of the bTableDef value.  Is that correct, or should that
> be in a block?  If it is correct, can you please also change the
> indentation of the nBorderTyp = XYZ; part so that it does not look as if
> it is supposed to be part of the if ( bTableDef )?

I'll to have a look at that... I can't remember the reason of this, but
it looks weird to me. IMHO setting it in all cases should be better...
but I need to dive again into RTF specs and that parser code (that I
partly rewrote some time ago)

-- 
Cédric Bosdonnat
LibreOffice hacker
http://documentfoundation.org
OOo Eclipse Integration developer
http://cedric.bosdonnat.free.fr