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

[Markus Mohrhard]

Hello Kendy, Cedric,

yes there might still be some problems, I only tried to fix that during
pasting to calc all borders are drawn. I'm not familiar with the RTF spec
and the old code(before Cedric refactored it) was really ugly and even used
some gotos.

This quick patch was more or less a result of
https://bugs.freedesktop.org/show_bug.cgi?id=37429 as I noticed that borders
were totally screwed up. If I can help in any way just mail me and I have a
look at it.

Regards, Markus

2011/5/27 Cedric Bosdonnat <cedric.bosdonnat.ooo at free.fr>

> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20110527/7296064a/attachment.html>