[PUSHED 3-5] Re: Bug 45522: rtf export, Crash when copying table between Writer and other LibreOffice programs

On 21/03/12 16:31, Caolán McNamara wrote:
> On Thu, 2012-03-08 at 17:23 +0100, Dézsi Szabolcs wrote:
> > Hi!
> >
> > https://bugs.freedesktop.org/show_bug.cgi?id=45522
> >
> > There's an attached odt file with a table in it. Copying the table to
> > other LO programs (Calc, etc) causes LO to crash.
> > After inspection I realized that the problem is in
> > sw/source/core/table/swtable.cxx
> > There is a struct definition in that file SwTableCellInfo::Impl with a
> > void setTable(const SwTable * pTable) method.
>
> So, I had a look at this as well. Here's what I think the situation
> is....
>
> The cut and paste goes through the writer rtf exporter, so we're talking
> about crashes in the rtf exporter. 
>
> Tables are miserably complicated in writer, and for complex tables I see
> that we try and take the layout information for the tables when
> available to use for exporting to rtf/doc. That only works if the
> document has been laid out though, and the temporary clipboard document
> isn't laid out, so yeah, that's going to return a NULL and crash
> horribly. Situation doesn't arise in .doc export as we don't cut and
> paste .doc format, but we do .rtf

ah yes... that stuff is "interesting"... so it used to be that there was
an "old-style" table model, where splitting and merging cells was
represented by having a cell include a nested sub-table.  this was
supported since forever by StarOffice and OOo up to 2.x.

but then it was discovered that there was a specific way of splitting up
a table that was impossible to represent in this table model, but was
somehow important (perhaps because MSO can do it?), and so a "new-style"
table model was designed that represented splitting and merging cells
with "row-span" and "col-span" attributes and "covered cell" elements.
this is the default table model since OOo 3.0,

but because of course there were customers with documents using the
old-style tables, the support for those is still retained; iirc most of
these are converted to new-style tables on ODF import, but in some cases
that is not possible or the result would look ugly or something, so the
Writer core actually supports both old-style and new-style tables, there
is just no UI support for creating an old-style table.

now it happened that a customer complained because their ODF document
containing old-style tables looked ugly when exported to DOC, and so
hbrinkm was tasked with implementing support for that, which he did by
doing the conversion via the layout frames (detecting which cells start
at the same height or something like that) and cursing a lot for a month
or two.  i think the result of that went into OOo 3.3.

now in OOo 3.4 of course vmiklos' new-fangled RTF export went in and
that uses the same ww8 code and the clipboard document doesn't have a
layout so that case crashes on copy/paste now.

> My first thought was a quick hack like the attached to use
> processSwTableByLayout only if the table has layout information and use
> the other branch otherwise.

eminently sensible approach, hence pushed to master and libreoffice-3-5.

> That gets us past the first crash, but then the rtf exporter breaks and
> falls over later on, possibly because its hard to get the table
> structure right without the layout information.

that would need some investigation for sure...

> I don't think we can call MakeFrms on a table to generate layout
> information in isolation without the full document being laid out (or
> can we?) so I guess we have to suck it down and fix the second layer of
> crashes in the .rtf exporter with the alternative table export branch.

i don't think MakeFrms will do anything at all but even if it doesn't
crash immediately the layout of just a table won't be correct.