[Libreoffice] [PUSHED] Re: [REVIEW][3-5] sequence of unfortunate writer ole object in headers problems

On 05/01/12 10:45, Caolán McNamara wrote:
> So https://bugs.freedesktop.org/show_bug.cgi?id=43867 is a most annoying
> bug with a crash on mailmerging documents that contain ole objects in
> headers. I'd like to get the following into 3-5.
>
> Immediate crash is fixed by
> http://cgit.freedesktop.org/libreoffice/core/commit/?id=7dc25878c1310a1de6d3c87c88404b151bad21fc
> where we previously acquired/released the document around the insertion
> of content into it, but not around inserting styles, but page styles can
> contain headers/footers with arbitrary content, so if we need to acquire
> before inserting content, we need to acquire before inserting page
> styles.

sounds plausible

> object preview is still missing from headers though,
> http://cgit.freedesktop.org/libreoffice/core/commit/?id=ef17be8b006737c078a59635ae334a03301727ea
> fixes a series of apparent errors
>
> 1. FN_UNO_REPLACEMENT_GRAPHIC_URL and FN_UNO_GRAPHIC have the same id in
> writer, so "replacement graphic urls" fall into the same case statement
> as "replacement graphics", which they aren't.

d'oh

> These random ids are *so*
> fragile, we need something better here in general.

indeed mis-merges seem to be pretty common with these.

since for UNO stuff usually a string is mapped to one of these integers,
i wonder if we could get rid of the mapping and rely on something like
OUString;:intern or OUString::hashCode for dispatch...

> 2. The replacement graphic is set with...
> svt::EmbeddedObjectRef xObj = pOleNode->GetOLEObj().GetObject();
> xObj.SetGraphic( aGraphic, aMediaType );
> but the x of xObj and the name "EmbeddedObjectRef" are misleading, its
> not a reference class, so we're setting the replacement graphic onto a
> temporary copy of the object. EmbeddedObjectRef is a poor class name, we
> should change that.

ouch.

> 3. SvXMLGraphicHelper::ImplGetStreamNames doesn't know how to
> split ./ObjectReplacements/foo in order to find the preview but
> SvXMLEmbeddedObjectHelper::ImplGetStorageNames does, so factored out
> better splitter and shared it 
>
> C.

have pushed these to libreoffice-3-5