text parts of shape exports

(Moving this part of the discussion to the list.)

On Fri, May 03, 2013 at 12:01:09PM +0000, bugzilla-daemon@freedesktop.org wrote:
> https://bugs.freedesktop.org/show_bug.cgi?id=64150

> --- Comment #2 from Markus Mohrhard <markus.mohrhard@googlemail.com> ---
> (In reply to comment #1)

> > The reproduction case document contains:

> > <text:p>
> >   <draw:frame text:anchor-type="paragraph" draw:style-name="gr1"
> >   draw:name="Object 10" draw:text-style-name="P1" svg:height="9cm"
> >   svg:width="14cm" svg:y="1cm" svg:x="1cm">
> >     <text:p draw:class-id="80243D39-6741-46C5-926e-069164ff87bb"></text:p>
> >     <draw:object-ole xlink:href="./Object" xlink:type="simple"
> >       xlink:show="embed" xlink:actuate="onLoad"/>
> >   </draw:frame>
> > </text:p>

> > as opposed to

> > <text:p>
> >   <draw:frame text:anchor-type="paragraph" draw:style-name="gr1"
> >     draw:name="Object 10" draw:text-style-name="P1" svg:height="9cm"
> >     svg:width="14cm" svg:y="1cm" svg:x="1cm">
> >     <draw:object-ole draw:class-id="80243D39-6741-46C5-926e-069164ff87bb"
> >        xlink:href="./Object" xlink:type="simple" xlink:show="embed"
> >        xlink:actuate="onLoad"/>
> >   </draw:frame>
> > </text:p>

> > At first sight, I thought this was invalid, since "that's not how it was
> > done before". But then, I stumbled on bug 58571, which seems to say that
> > this kind of construct is valid for most shapes, but not for charts. But I
> > don't find where in the ODF specification a <draw:frame> is allowed to have
> > a <text:p> child, except if that <text:p> child "refers to a chart". So I'm
> > confused.

> I did not want to apply with my bug fix that it is valid for any
> other case.  I'm just not familiar enough with the shape export to
> disable it for anything else than the chart case where I know that
> it is invalid.

Anybody else on the ML wants to review the situation for other shapes?

> The chart objects where it is allowed are not represented by the two
> values for which I disabled the export.

So it is allowed for *some* chart objects?

Also, if the value XmlShapeTypeDrawChartShape correspond to "no text
allowed", shouldn't we also revert part of this hunk from commit
17ff7b41d15ab9928e2e2706faa26234a09802cd ?


diff --git a/xmloff/source/draw/shapeexport.cxx b/xmloff/source/draw/shapeexport.cxx
index 606164b..cfb5649 100644
--- a/xmloff/source/draw/shapeexport.cxx
+++ b/xmloff/source/draw/shapeexport.cxx
@@ -219,10 +210,10 @@ void
XMLShapeExport::collectShapeAutoStyles(const uno::Reference<drawing::XShap
     // -----------------------------
     ImpCalcShapeType(xShape, aShapeInfo.meShapeType);
 
+    // #i118485# enabled XmlShapeTypeDrawChartShape and XmlShapeTypeDrawOLE2Shape
+    // to have text
     const bool bObjSupportsText =
-        aShapeInfo.meShapeType != XmlShapeTypeDrawChartShape &&
         aShapeInfo.meShapeType != XmlShapeTypePresChartShape &&
-        aShapeInfo.meShapeType != XmlShapeTypeDrawOLE2Shape &&
         aShapeInfo.meShapeType != XmlShapeTypePresOLE2Shape &&
         aShapeInfo.meShapeType != XmlShapeTypeDrawSheetShape &&
         aShapeInfo.meShapeType != XmlShapeTypePresSheetShape &&

> > On the contrary,
> > http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part1.
> > html#__RefHeading__1415848_253892949 says:

> > The <draw:frame> element has the following child elements: (...)

> So you have the same case that I had and you should put a breakpoint
> into that method and check that you can safely disable the text:p
> export for it.

So, I changed report charts to map to value
XmlShapeTypeDrawChartShape, which has the same effect. I don't even
understand why they have a different CLSID, actually... They look like
plain draw charts to me.

-- 
Lionel