sd/qa/unit/export-tests-ooxml4.cxx | 5 ++
sd/source/ui/view/drviews2.cxx | 7 +--
svtools/source/misc/embedhlp.cxx | 63 ++++++---------------------
svx/source/svdraw/svdedtv2.cxx | 11 +++-
svx/source/svdraw/svdxcgv.cxx | 4 -
svx/source/unodraw/unoshap4.cxx | 2
svx/source/unodraw/unoshape.cxx | 2
sw/qa/core/view/view.cxx | 5 +-
sw/source/core/docnode/ndnotxt.cxx | 3 -
sw/source/filter/html/htmlplug.cxx | 11 +---
sw/source/filter/html/htmlreqifreader.cxx | 3 -
sw/source/filter/ww8/docxattributeoutput.cxx | 7 +--
sw/source/filter/ww8/rtfattributeoutput.cxx | 7 +--
13 files changed, 52 insertions(+), 78 deletions(-)
New commits:
commit 7749f931389ada7f25e3c1d52c1a760ec41a3019
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Fri Sep 20 14:43:55 2024 +0500
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Sat Sep 21 14:57:52 2024 +0200
Make replacement graphic management more atomic
Commit 8872f7121b4ae4dd0b51820366d3510a88f7aac2 (crashtesting: crash
on exporting kde274105-6.docx to .rtf, 2024-03-27) added some safety
code in EmbeddedObjectRef::GetReplacement. It mentioned, that there
are likely some bugs in the management of the graphic.
This tries to fix this management, avoiding the intermediate states,
and only changing the graphic when all the data is ready. This also
reverts the changes of the mentioned commit, obsoleted now; and of
commit 8780fa41dcd164af244742461f4e57a4bcf4c7a4 (svtools: fix lost
replacement grpahic when updating it via OLE fails, 2018-10-30);
but keeps commit 24231f2a336c50eac5e6a1621c57e60ebe1e1cf4 (svtools:
fix lost replacement non-rendered graphic when updating it fails,
2022-02-17).
This has revealed that the second part of unit test for tdf#143222
("Check export of embedded worksheet in slide"), introduced in
commit 92a407b7f90a98704a238c5ffa3a3491eaf3263a (tdf143222 Handle
alternate content of graphicData element., 2021-07-08), has never
really worked: the "pGraphic != nullptr" check would never fail;
in fact, that used to always return an empty graphic. The problem
was filed as tdf#163064, and the test was modified accordingly.
Commit 5d997c029e53c31a3651a08f5012645097cec48f (sw XHTML export:
improve dummy OLE object handling, 2018-08-30) made ReqIF export
handle missing replacement graphic. However, it had assumed that
SwOLENode::GetGraphic always returns a valid pointer even for the
missing data. That is fixed here in OutHTML_FrameFormatOLENodeGrf.
Other places, where the pointer was dereferenced unconditionally,
were fixed (keeping current behavior).
Change-Id: Ica97a691ecc11b856bdb003f89467ea3392684bd
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/173716
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/sd/qa/unit/export-tests-ooxml4.cxx
b/sd/qa/unit/export-tests-ooxml4.cxx
index 0382792afe45..bac50f28bdc1 100644
--- a/sd/qa/unit/export-tests-ooxml4.cxx
+++ b/sd/qa/unit/export-tests-ooxml4.cxx
@@ -732,7 +732,10 @@ CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest4,
testTdf143222_embeddedWorksheet)
// Without the fix we lost the graphic of ole object.
const Graphic* pGraphic = pOleObj->GetGraphic();
CPPUNIT_ASSERT_MESSAGE("no graphic", pGraphic != nullptr);
+ CPPUNIT_ASSERT_MESSAGE("no graphic", !pGraphic->IsNone());
+ // TODO: this currently fails - see tdf#163064
+#if 0
// Check export of embedded worksheet in slide.
saveAndReload(u"Impress Office Open XML"_ustr);
@@ -742,6 +745,8 @@ CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest4,
testTdf143222_embeddedWorksheet)
pGraphic = pOleObj->GetGraphic();
CPPUNIT_ASSERT_MESSAGE("no graphic after the export", pGraphic != nullptr);
+ CPPUNIT_ASSERT_MESSAGE("no graphic after the export", !pGraphic->IsNone());
+#endif
}
CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest4, testTdf143315)
diff --git a/sd/source/ui/view/drviews2.cxx b/sd/source/ui/view/drviews2.cxx
index cd60935b9c06..ded35227175d 100644
--- a/sd/source/ui/view/drviews2.cxx
+++ b/sd/source/ui/view/drviews2.cxx
@@ -2989,10 +2989,9 @@ void DrawViewShell::FuTemporary(SfxRequest& rReq)
}
}
- if(pOle2 && pOle2->GetGraphic())
- {
- nCount +=
pOle2->GetGraphic()->GetGDIMetaFile().GetActionSize();
- }
+ if (pOle2)
+ if (const Graphic* pGraphic = pOle2->GetGraphic())
+ nCount +=
pGraphic->GetGDIMetaFile().GetActionSize();
}
// decide with the sum of all meta objects if we should show a
dialog
diff --git a/svtools/source/misc/embedhlp.cxx b/svtools/source/misc/embedhlp.cxx
index 9e14fb5d37a8..78b5909f1602 100644
--- a/svtools/source/misc/embedhlp.cxx
+++ b/svtools/source/misc/embedhlp.cxx
@@ -452,37 +452,21 @@ const Link<LinkParamNone*, bool> &
EmbeddedObjectRef::GetIsProtectedHdl() const
void EmbeddedObjectRef::GetReplacement( bool bUpdate )
{
- Graphic aOldGraphic;
- OUString aOldMediaType;
-
if ( bUpdate )
{
- if (mpImpl->oGraphic)
- aOldGraphic = *mpImpl->oGraphic;
- aOldMediaType = mpImpl->aMediaType;
-
// Do not clear / reset mpImpl->oGraphic, because it would appear as
no replacement
// on any call to getReplacementGraphic during the external calls to
the OLE object,
// which may release mutexes. Only replace it when done.
mpImpl->aMediaType.clear();
- mpImpl->mnGraphicVersion++;
- }
- else if ( !mpImpl->oGraphic )
- {
- mpImpl->mnGraphicVersion++;
}
- else
+ else if (mpImpl->oGraphic)
{
OSL_FAIL("No update, but replacement exists already!");
return;
}
- // Missing graphic can crash
- if (!mpImpl->oGraphic)
- mpImpl->oGraphic.emplace();
-
std::unique_ptr<SvStream> pGraphicStream(GetGraphicStream( bUpdate ));
- if (!pGraphicStream && aOldGraphic.IsNone())
+ if (!pGraphicStream && bUpdate && (!mpImpl->oGraphic ||
mpImpl->oGraphic->IsNone()))
{
// We have no old graphic, tried to get an updated one, but that
failed. Try to get an old
// graphic instead of having no graphic at all.
@@ -491,31 +475,16 @@ void EmbeddedObjectRef::GetReplacement( bool bUpdate )
"EmbeddedObjectRef::GetReplacement: failed to get updated
graphic stream");
}
- if (!mpImpl->oGraphic)
- {
- // note that UpdateReplacementOnDemand which resets mpImpl->oGraphic
to null may have been called
- // e.g. when exporting ooo58458-1.odt to doc or kde274105-6.docx to
rtf. Those looks like bugs as
- // presumably generating the replacement graphic shouldn't re-trigger
that the graphic needs
- // to be updated, bodge this to work as callers naturally expect
- SAL_WARN("svtools.misc", "EmbeddedObjectRef::GetReplacement generating
replacement image modified object to claim it needs to update replacement");
- mpImpl->oGraphic.emplace();
- }
-
if ( pGraphicStream )
{
GraphicFilter& rGF = GraphicFilter::GetGraphicFilter();
- rGF.ImportGraphic( *mpImpl->oGraphic, u"", *pGraphicStream );
- mpImpl->mnGraphicVersion++;
- }
-
- if (bUpdate && mpImpl->oGraphic->IsNone() && !aOldGraphic.IsNone())
- {
- // We used to have an old graphic, tried to update and the update
- // failed. Go back to the old graphic instead of having no graphic at
- // all.
- mpImpl->oGraphic.emplace(aOldGraphic);
- mpImpl->aMediaType = aOldMediaType;
- SAL_WARN("svtools.misc", "EmbeddedObjectRef::GetReplacement: failed to
update graphic");
+ Graphic aNewGraphic;
+ rGF.ImportGraphic(aNewGraphic, u"", *pGraphicStream);
+ if (!aNewGraphic.IsNone())
+ {
+ mpImpl->oGraphic.emplace(aNewGraphic);
+ mpImpl->mnGraphicVersion++;
+ }
}
}
@@ -601,17 +570,13 @@ Size EmbeddedObjectRef::GetSize( MapMode const *
pTargetMapMode ) const
void EmbeddedObjectRef::SetGraphicStream( const uno::Reference<
io::XInputStream >& xInGrStream,
const OUString& rMediaType )
{
- mpImpl->oGraphic.emplace();
- mpImpl->aMediaType = rMediaType;
- mpImpl->mnGraphicVersion++;
-
+ Graphic aNewGraphic;
std::unique_ptr<SvStream>
pGraphicStream(::utl::UcbStreamHelper::CreateStream( xInGrStream ));
if ( pGraphicStream )
{
GraphicFilter& rGF = GraphicFilter::GetGraphicFilter();
- rGF.ImportGraphic( *mpImpl->oGraphic, u"", *pGraphicStream );
- mpImpl->mnGraphicVersion++;
+ rGF.ImportGraphic(aNewGraphic, u"", *pGraphicStream);
if ( mpImpl->pContainer )
{
@@ -622,8 +587,10 @@ void EmbeddedObjectRef::SetGraphicStream( const
uno::Reference< io::XInputStream
}
}
+ mpImpl->oGraphic.emplace(aNewGraphic);
+ mpImpl->aMediaType = rMediaType;
+ mpImpl->mnGraphicVersion++;
mpImpl->bNeedUpdate = false;
-
}
void EmbeddedObjectRef::SetGraphic( const Graphic& rGraphic, const OUString&
rMediaType )
@@ -955,9 +922,7 @@ void EmbeddedObjectRef::UpdateOleObject( bool bUpdateOle )
void EmbeddedObjectRef::UpdateReplacementOnDemand()
{
- mpImpl->oGraphic.reset();
mpImpl->bNeedUpdate = true;
- mpImpl->mnGraphicVersion++;
if( mpImpl->pContainer )
{
diff --git a/svx/source/svdraw/svdedtv2.cxx b/svx/source/svdraw/svdedtv2.cxx
index 4f82ec7eb83c..912e145cef62 100644
--- a/svx/source/svdraw/svdedtv2.cxx
+++ b/svx/source/svdraw/svdedtv2.cxx
@@ -2185,11 +2185,14 @@ void SdrEditView::DoImportMarkedMtf(SvdProgressInfo
*pProgrInfo)
}
SdrOle2Obj* pOle2 = dynamic_cast<SdrOle2Obj*>(pObj);
- if (pOle2 != nullptr && pOle2->GetGraphic())
+ if (pOle2)
{
- aLogicRect = pOle2->GetLogicRect();
- ImpSdrGDIMetaFileImport aFilter(GetModel(), pObj->GetLayer(),
aLogicRect);
- nInsCnt = aFilter.DoImport(pOle2->GetGraphic()->GetGDIMetaFile(),
*pOL, nInsPos, pProgrInfo);
+ if (const Graphic* pGraphic = pOle2->GetGraphic())
+ {
+ aLogicRect = pOle2->GetLogicRect();
+ ImpSdrGDIMetaFileImport aFilter(GetModel(), pObj->GetLayer(),
aLogicRect);
+ nInsCnt = aFilter.DoImport(pGraphic->GetGDIMetaFile(), *pOL,
nInsPos, pProgrInfo);
+ }
}
if (nInsCnt != 0)
diff --git a/svx/source/svdraw/svdxcgv.cxx b/svx/source/svdraw/svdxcgv.cxx
index e3ad1a0833a8..ab4fa320275c 100644
--- a/svx/source/svdraw/svdxcgv.cxx
+++ b/svx/source/svdraw/svdxcgv.cxx
@@ -611,9 +611,9 @@ Graphic SdrExchangeView::GetObjGraphic(const SdrObject&
rSdrObject, bool bSVG)
}
else if (pSdrOle2Obj)
{
- if (pSdrOle2Obj->GetGraphic())
+ if (const Graphic* pGraphic = pSdrOle2Obj->GetGraphic())
{
- aRet = *pSdrOle2Obj->GetGraphic();
+ aRet = *pGraphic;
}
}
else
diff --git a/svx/source/unodraw/unoshap4.cxx b/svx/source/unodraw/unoshap4.cxx
index ab281ad4e674..4607482cc532 100644
--- a/svx/source/unodraw/unoshap4.cxx
+++ b/svx/source/unodraw/unoshap4.cxx
@@ -243,7 +243,7 @@ bool SvxOle2Shape::getPropertyValueImpl( const OUString&
rName, const SfxItemPro
if ( !bIsWMF )
{
// #i119735# just use GetGDIMetaFile, it will create a
buffered version of contained bitmap now automatically
- GDIMetaFile aMtf(pObj->GetGraphic()->GetGDIMetaFile());
+ GDIMetaFile aMtf(pGraphic->GetGDIMetaFile());
SvMemoryStream aDestStrm( 65535, 65535 );
ConvertGDIMetaFileToWMF( aMtf, aDestStrm, nullptr, false );
const uno::Sequence<sal_Int8> aSeq(
diff --git a/svx/source/unodraw/unoshape.cxx b/svx/source/unodraw/unoshape.cxx
index b9878ffd2998..04171a9ae81b 100644
--- a/svx/source/unodraw/unoshape.cxx
+++ b/svx/source/unodraw/unoshape.cxx
@@ -2918,7 +2918,7 @@ bool SvxShape::getPropertyValueImpl( const OUString&,
const SfxItemPropertyMapEn
if ( !bIsWMF )
{
// #119735# just use GetGDIMetaFile, it will create a
buffered version of contained bitmap now automatically
- GDIMetaFile aMtf(pObj->GetGraphic()->GetGDIMetaFile());
+ GDIMetaFile aMtf(pGraphic->GetGDIMetaFile());
SvMemoryStream aDestStrm( 65535, 65535 );
ConvertGDIMetaFileToWMF( aMtf, aDestStrm, nullptr, false );
const uno::Sequence<sal_Int8> aSeq(
diff --git a/sw/qa/core/view/view.cxx b/sw/qa/core/view/view.cxx
index 2c6655a659b7..b130827deb8d 100644
--- a/sw/qa/core/view/view.cxx
+++ b/sw/qa/core/view/view.cxx
@@ -60,10 +60,11 @@ CPPUNIT_TEST_FIXTURE(Test, testUpdateOleObjectPreviews)
SwOLENode* pOleNode = pNode->GetOLENode();
CPPUNIT_ASSERT(pOleNode);
SwOLEObj& rOleObj = pOleNode->GetOLEObj();
- svt::EmbeddedObjectRef& rObject = rOleObj.GetObject();
+ const Graphic* pGraphic = rOleObj.GetObject().GetGraphic();
// Without the accompanying fix in place, this test would have failed,
the update broke the
// preview of the second embedded object.
- CPPUNIT_ASSERT(!rObject.GetGraphic()->IsNone());
+ CPPUNIT_ASSERT(pGraphic);
+ CPPUNIT_ASSERT(!pGraphic->IsNone());
}
}
diff --git a/sw/source/core/docnode/ndnotxt.cxx
b/sw/source/core/docnode/ndnotxt.cxx
index d044f4e90dd3..0fdb56751023 100644
--- a/sw/source/core/docnode/ndnotxt.cxx
+++ b/sw/source/core/docnode/ndnotxt.cxx
@@ -231,7 +231,8 @@ Graphic SwNoTextNode::GetGraphic() const
else
{
OSL_ENSURE( GetOLENode(), "new type of Node?" );
- aRet = *const_cast<SwOLENode*>(static_cast<const
SwOLENode*>(this))->SwOLENode::GetGraphic();
+ if (const Graphic* pGraphic = const_cast<SwOLENode*>(static_cast<const
SwOLENode*>(this))->SwOLENode::GetGraphic())
+ aRet = *pGraphic;
}
return aRet;
}
diff --git a/sw/source/filter/html/htmlplug.cxx
b/sw/source/filter/html/htmlplug.cxx
index 3e594bbf8b43..238b09dbc2dc 100644
--- a/sw/source/filter/html/htmlplug.cxx
+++ b/sw/source/filter/html/htmlplug.cxx
@@ -1649,13 +1649,10 @@ SwHTMLWriter& OutHTML_FrameFormatOLENodeGrf(
SwHTMLWriter& rWrt, const SwFrameFo
if (TrySaveFormulaAsPDF(rWrt, rFrameFormat, pOLENd,
bWriteReplacementGraphic, bInCntnr))
return rWrt;
- if ( !pOLENd->GetGraphic() )
- {
- SAL_WARN("sw.html", "Unexpected missing OLE fallback graphic");
- return rWrt;
- }
-
- Graphic aGraphic( *pOLENd->GetGraphic() );
+ // Missing fallback graphic must not give up exporting the document, and
also must produce
+ // valid output (see OutHTMLGraphic)
+ const Graphic* pFallbackGraphic = pOLENd->GetGraphic();
+ Graphic aGraphic(pFallbackGraphic ? *pFallbackGraphic : Graphic());
SwDocShell* pDocSh = rWrt.m_pDoc->GetDocShell();
bool bObjectOpened = false;
diff --git a/sw/source/filter/html/htmlreqifreader.cxx
b/sw/source/filter/html/htmlreqifreader.cxx
index d0c059037231..9493be474282 100644
--- a/sw/source/filter/html/htmlreqifreader.cxx
+++ b/sw/source/filter/html/htmlreqifreader.cxx
@@ -223,7 +223,8 @@ OString InsertOLE1HeaderFromOle10NativeStream(const
rtl::Reference<SotStorage>&
return aClassName;
}
- const Graphic& rGraphic = *rOLENode.GetGraphic();
+ const Graphic* pGraphic = rOLENode.GetGraphic();
+ const Graphic rGraphic = pGraphic ? *pGraphic : Graphic();
Size aSize = rOLENode.GetTwipSize();
SvMemoryStream aGraphicStream;
if (GraphicConverter::Export(aGraphicStream, rGraphic,
ConvertDataFormat::WMF) != ERRCODE_NONE)
diff --git a/sw/source/filter/ww8/docxattributeoutput.cxx
b/sw/source/filter/ww8/docxattributeoutput.cxx
index 60646c0f2a72..6b38d40f1ad5 100644
--- a/sw/source/filter/ww8/docxattributeoutput.cxx
+++ b/sw/source/filter/ww8/docxattributeoutput.cxx
@@ -5226,8 +5226,8 @@ void DocxAttributeOutput::FlyFrameGraphic( const
SwGrfNode* pGrfNode, const Size
Graphic aGraphic;
if (pGrfNode)
aGraphic = pGrfNode->GetGrf();
- else
- aGraphic = *pOLENode->GetGraphic();
+ else if (const Graphic* pGraphic = pOLENode->GetGraphic())
+ aGraphic = *pGraphic;
m_rDrawingML.SetFS(m_pSerializer); // to be sure that we write to the
right stream
auto pGraphicExport = m_rDrawingML.createGraphicExport();
@@ -5876,8 +5876,9 @@ void DocxAttributeOutput::WriteOLE( SwOLENode& rNode,
const Size& rSize, const S
// write preview image
const Graphic* pGraphic = rNode.GetGraphic();
+ Graphic aGraphic = pGraphic ? *pGraphic : Graphic();
m_rDrawingML.SetFS(m_pSerializer);
- OUString sImageId = m_rDrawingML.writeGraphicToStorage(*pGraphic);
+ OUString sImageId = m_rDrawingML.writeGraphicToStorage(aGraphic);
if ( sDrawAspect == "Content" )
{
diff --git a/sw/source/filter/ww8/rtfattributeoutput.cxx
b/sw/source/filter/ww8/rtfattributeoutput.cxx
index 2a910cc9aced..dd0d581257fa 100644
--- a/sw/source/filter/ww8/rtfattributeoutput.cxx
+++ b/sw/source/filter/ww8/rtfattributeoutput.cxx
@@ -4389,12 +4389,13 @@ void RtfAttributeOutput::FlyFrameOLEReplacement(const
SwFlyFrameFormat* pFlyFram
aRendered.setWidth(rSize.Width());
aRendered.setHeight(rSize.Height());
const Graphic* pGraphic = rOLENode.GetGraphic();
- Size aMapped(pGraphic->GetPrefSize());
+ Graphic aGraphic = pGraphic ? *pGraphic : Graphic();
+ Size aMapped(aGraphic.GetPrefSize());
auto& rCr = rOLENode.GetAttr(RES_GRFATR_CROPGRF);
const char* pBLIPType = OOO_STRING_SVTOOLS_RTF_PNGBLIP;
const sal_uInt8* pGraphicAry = nullptr;
SvMemoryStream aStream;
- if (GraphicConverter::Export(aStream, *pGraphic, ConvertDataFormat::PNG)
!= ERRCODE_NONE)
+ if (GraphicConverter::Export(aStream, aGraphic, ConvertDataFormat::PNG) !=
ERRCODE_NONE)
SAL_WARN("sw.rtf", "failed to export the graphic");
sal_uInt64 nSize = aStream.TellEnd();
pGraphicAry = static_cast<sal_uInt8 const*>(aStream.GetData());
@@ -4404,7 +4405,7 @@ void RtfAttributeOutput::FlyFrameOLEReplacement(const
SwFlyFrameFormat* pFlyFram
m_aRunText->append("{" OOO_STRING_SVTOOLS_RTF_NONSHPPICT);
pBLIPType = OOO_STRING_SVTOOLS_RTF_WMETAFILE;
SvMemoryStream aWmfStream;
- if (GraphicConverter::Export(aWmfStream, *pGraphic,
ConvertDataFormat::WMF) != ERRCODE_NONE)
+ if (GraphicConverter::Export(aWmfStream, aGraphic, ConvertDataFormat::WMF)
!= ERRCODE_NONE)
SAL_WARN("sw.rtf", "failed to export the graphic");
nSize = aWmfStream.TellEnd();
pGraphicAry = static_cast<sal_uInt8 const*>(aWmfStream.GetData());
