embeddedobj/source/commonembedding/embedobj.cxx | 8 +- embeddedobj/source/commonembedding/miscobj.cxx | 68 +++++++++++++++++- embeddedobj/source/commonembedding/persistence.cxx | 78 +++++++++++++++------ embeddedobj/source/commonembedding/visobj.cxx | 8 +- embeddedobj/source/inc/commonembobj.hxx | 7 + 5 files changed, 141 insertions(+), 28 deletions(-)
New commits: commit aeca771f8a8e4c77008e921eebcdb5784003e419 Author: Armin Le Grand (Allotropia) <armin.le.gr...@me.com> AuthorDate: Thu Apr 8 10:30:15 2021 +0200 Commit: Adolfo Jayme Barrientos <fit...@ubuntu.com> CommitDate: Thu Apr 8 21:07:05 2021 +0200 tdf#141529 sync linked OLE save(s) with hosting file save(s) Before deactivating a linked OLE saved it and overwrote the original, even when not saving the hosting document at all. This is not intuitive from user perspective and may lead to unexpected data loss (of the OLE content). Reported case was especially about closing the hosted document without saving in the understandable believe that that way the changed OLE will not be changed on external medium. Added mechanism for linked OLE to hold data in a hidden local temp file, synching/writing back on hosting file save. Most complicated was adapting the 'break link' case and ensuring other cases to work, but looks good now from my POV Change-Id: I7f63d667820b2d9725abc598a9dd7360be1f8d5a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/113793 Tested-by: Armin Le Grand <armin.le.gr...@me.com> Reviewed-by: Armin Le Grand <armin.le.gr...@me.com> (cherry picked from commit f4c66a2272d890eaa7a6d7429e5bfa7bdb1f7162) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/113762 Tested-by: Jenkins Reviewed-by: Adolfo Jayme Barrientos <fit...@ubuntu.com> diff --git a/embeddedobj/source/commonembedding/embedobj.cxx b/embeddedobj/source/commonembedding/embedobj.cxx index 669ba884b055..ffa2a0789be0 100644 --- a/embeddedobj/source/commonembedding/embedobj.cxx +++ b/embeddedobj/source/commonembedding/embedobj.cxx @@ -100,6 +100,12 @@ void OCommonEmbeddedObject::Deactivate() { try { xClientSite->saveObject(); + + // tdf#141529 take note that an eventually used linked file + // got changed/saved/written and that we need to copy it back if the + // hosting file/document gets saved + if(m_aLinkTempFile.is()) + m_bLinkTempFileChanged = true; } catch( const embed::ObjectSaveVetoException& ) { @@ -168,7 +174,7 @@ void OCommonEmbeddedObject::SwitchStateTo_Impl( sal_Int32 nNextState ) // after the object reaches the running state the cloned size is not necessary any more m_bHasClonedSize = false; - if ( m_bIsLink ) + if ( m_bIsLinkURL ) { m_xDocHolder->SetComponent( LoadLink_Impl(), m_bReadOnly ); } diff --git a/embeddedobj/source/commonembedding/miscobj.cxx b/embeddedobj/source/commonembedding/miscobj.cxx index ea2a3cb3c110..3d0cbb98b839 100644 --- a/embeddedobj/source/commonembedding/miscobj.cxx +++ b/embeddedobj/source/commonembedding/miscobj.cxx @@ -27,6 +27,10 @@ #include <com/sun/star/lang/NoSupportException.hpp> #include <com/sun/star/beans/NamedValue.hpp> +#include <com/sun/star/ucb/SimpleFileAccess.hpp> +#include <com/sun/star/io/TempFile.hpp> +#include <comphelper/storagehelper.hxx> + #include <cppuhelper/queryinterface.hxx> #include <cppuhelper/interfacecontainer.h> #include <comphelper/mimeconfighelper.hxx> @@ -52,8 +56,10 @@ OCommonEmbeddedObject::OCommonEmbeddedObject( const uno::Reference< uno::XCompon , m_bEmbeddedScriptSupport( true ) , m_bDocumentRecoverySupport( true ) , m_bWaitSaveCompleted( false ) -, m_bIsLink( false ) +, m_bIsLinkURL( false ) +, m_bLinkTempFileChanged( false ) , m_bLinkHasPassword( false ) +, m_aLinkTempFile( ) , m_bHasClonedSize( false ) , m_nClonedMapUnit( 0 ) { @@ -78,8 +84,10 @@ OCommonEmbeddedObject::OCommonEmbeddedObject( , m_bEmbeddedScriptSupport( true ) , m_bDocumentRecoverySupport( true ) , m_bWaitSaveCompleted( false ) -, m_bIsLink( true ) +, m_bIsLinkURL( true ) +, m_bLinkTempFileChanged( false ) , m_bLinkHasPassword( false ) +, m_aLinkTempFile( ) , m_bHasClonedSize( false ) , m_nClonedMapUnit( 0 ) { @@ -214,7 +222,61 @@ void OCommonEmbeddedObject::LinkInit_Impl( m_bReadOnly = aExportFilterName != m_aLinkFilterName; } - m_aDocMediaDescriptor = GetValuableArgs_Impl( aMediaDescr, false ); + if(m_bIsLinkURL && !m_bReadOnly) + { + // tdf#141529 we have a linked OLE object. To prevent the original OLE + // data to be changed each time the OLE gets changed (at deactivate), copy it to + // a temporary file. That file will be changed on activated OLE changes then. + // The moment the original gets changed itself will now be associated with the + // file/document embedding the OLE being changed (see other additions to the + // task-ID above) + // + // open OLE original data as read input file + uno::Reference< ucb::XSimpleFileAccess3 > xTempAccess( ucb::SimpleFileAccess::create( m_xContext ) ); + uno::Reference< io::XInputStream > xInStream( xTempAccess->openFileRead( m_aLinkURL ) ); + + if(xInStream.is()) + { + // create temporary file + m_aLinkTempFile = io::TempFile::create(m_xContext); + + if(m_aLinkTempFile.is()) + { + // completely copy content of original OLE data + uno::Reference < io::XOutputStream > xTempOut = m_aLinkTempFile->getOutputStream(); + ::comphelper::OStorageHelper::CopyInputToOutput( xInStream, xTempOut ); + xTempOut->flush(); + xTempOut->closeOutput(); + + // reset flag m_bLinkTempFileChanged, so it will also work for multiple + // save op's of the containing file/document + m_bLinkTempFileChanged = false; + } + } + } + + if(m_aLinkTempFile.is()) + { + uno::Sequence< beans::PropertyValue > aAlternativeMediaDescr(aMediaDescr.getLength()); + + for ( sal_Int32 a(0); a < aMediaDescr.getLength(); a++ ) + { + const beans::PropertyValue& rSource(aMediaDescr[a]); + beans::PropertyValue& rDestination(aAlternativeMediaDescr[a]); + + rDestination.Name = rSource.Name; + if(rSource.Name == "URL") + rDestination.Value <<= m_aLinkTempFile->getUri(); + else + rDestination.Value = rSource.Value; + } + + m_aDocMediaDescriptor = GetValuableArgs_Impl( aAlternativeMediaDescr, false ); + } + else + { + m_aDocMediaDescriptor = GetValuableArgs_Impl( aMediaDescr, false ); + } uno::Reference< frame::XDispatchProviderInterceptor > xDispatchInterceptor; for ( beans::PropertyValue const & prop : aObjectDescr ) diff --git a/embeddedobj/source/commonembedding/persistence.cxx b/embeddedobj/source/commonembedding/persistence.cxx index bf0bcdf2bbf2..d21eb297360a 100644 --- a/embeddedobj/source/commonembedding/persistence.cxx +++ b/embeddedobj/source/commonembedding/persistence.cxx @@ -47,6 +47,9 @@ #include <com/sun/star/beans/IllegalTypeException.hpp> #include <com/sun/star/chart2/XChartDocument.hpp> +#include <com/sun/star/ucb/SimpleFileAccess.hpp> +#include <com/sun/star/io/XTruncate.hpp> + #include <comphelper/fileformat.h> #include <comphelper/storagehelper.hxx> #include <comphelper/mimeconfighelper.hxx> @@ -254,7 +257,7 @@ void OCommonEmbeddedObject::SwitchOwnPersistence( const uno::Reference< embed::X m_aEntryName = aNewName; // the linked document should not be switched - if ( !m_bIsLink ) + if ( !m_bIsLinkURL ) { uno::Reference< document::XStorageBasedDocument > xDoc( m_xDocHolder->GetComponent(), uno::UNO_QUERY ); if ( xDoc.is() ) @@ -369,10 +372,16 @@ uno::Reference< util::XCloseable > OCommonEmbeddedObject::LoadLink_Impl() sal_Int32 nLen = 2; uno::Sequence< beans::PropertyValue > aArgs( nLen ); + aArgs[0].Name = "URL"; - aArgs[0].Value <<= m_aLinkURL; + if(m_aLinkTempFile.is()) + aArgs[0].Value <<= m_aLinkTempFile->getUri(); + else + aArgs[0].Value <<= m_aLinkURL; + aArgs[1].Name = "FilterName"; aArgs[1].Value <<= m_aLinkFilterName; + if ( m_bLinkHasPassword ) { aArgs.realloc( ++nLen ); @@ -843,7 +852,7 @@ uno::Reference< util::XCloseable > OCommonEmbeddedObject::CreateTempDocFromLink_ { uno::Reference< util::XCloseable > xResult; - SAL_WARN_IF( !m_bIsLink, "embeddedobj.common", "The object is not a linked one!" ); + SAL_WARN_IF( !m_bIsLinkURL, "embeddedobj.common", "The object is not a linked one!" ); uno::Sequence< beans::PropertyValue > aTempMediaDescr; @@ -895,7 +904,12 @@ uno::Reference< util::XCloseable > OCommonEmbeddedObject::CreateTempDocFromLink_ { aTempMediaDescr.realloc( 2 ); aTempMediaDescr[0].Name = "URL"; - aTempMediaDescr[0].Value <<= m_aLinkURL; + + // tdf#141529 use URL of the linked TempFile if it exists + aTempMediaDescr[0].Value <<= m_aLinkTempFile.is() + ? m_aLinkTempFile->getUri() + : m_aLinkURL; + aTempMediaDescr[1].Name = "FilterName"; aTempMediaDescr[1].Value <<= m_aLinkFilterName; } @@ -968,8 +982,8 @@ void SAL_CALL OCommonEmbeddedObject::setPersistentEntry( // for now support of this interface is required to allow breaking of links and converting them to normal embedded // objects, so the persist name must be handled correctly ( althowgh no real persist entry is used ) - // OSL_ENSURE( !m_bIsLink, "This method implementation must not be used for links!" ); - if ( m_bIsLink ) + // OSL_ENSURE( !m_bIsLinkURL, "This method implementation must not be used for links!" ); + if ( m_bIsLinkURL ) { m_aEntryName = sEntName; return; @@ -1147,8 +1161,8 @@ void SAL_CALL OCommonEmbeddedObject::storeToEntry( const uno::Reference< embed:: // for now support of this interface is required to allow breaking of links and converting them to normal embedded // objects, so the persist name must be handled correctly ( althowgh no real persist entry is used ) - // OSL_ENSURE( !m_bIsLink, "This method implementation must not be used for links!" ); - if ( m_bIsLink ) + // OSL_ENSURE( !m_bIsLinkURL, "This method implementation must not be used for links!" ); + if ( m_bIsLinkURL ) return; OSL_ENSURE( m_xParentStorage.is() && m_xObjectStorage.is(), "The object has no valid persistence!" ); @@ -1275,10 +1289,33 @@ void SAL_CALL OCommonEmbeddedObject::storeAsEntry( const uno::Reference< embed:: // for now support of this interface is required to allow breaking of links and converting them to normal embedded // objects, so the persist name must be handled correctly ( althowgh no real persist entry is used ) - // OSL_ENSURE( !m_bIsLink, "This method implementation must not be used for links!" ); - if ( m_bIsLink ) + // OSL_ENSURE( !m_bIsLinkURL, "This method implementation must not be used for links!" ); + if ( m_bIsLinkURL ) { m_aNewEntryName = sEntName; + + if(m_aLinkTempFile.is() && m_bLinkTempFileChanged) + { + // tdf#141529 if we have a changed copy of the original OLE data we now + // need to write it back 'over' the original OLE data + uno::Reference < ucb::XSimpleFileAccess2 > xFileAccess(ucb::SimpleFileAccess::create( m_xContext )); + uno::Reference < io::XInputStream > xTempIn = m_aLinkTempFile->getInputStream(); + + // This is *needed* since OTempFileService calls OTempFileService::readBytes which + // ensures the SvStream mpStream gets/is opened, *but* also sets the mnCachedPos from + // OTempFileService which still points to the end-of-file (from write-cc'ing). + uno::Reference < io::XSeekable > xSeek( xTempIn, uno::UNO_QUERY_THROW ); + xSeek->seek(0); + + xFileAccess->writeFile(m_aLinkURL, xTempIn); + + // Do *not* close input, that would remove the temporary file too early + // xTempIn->closeInput(); + + // reset flag m_bLinkTempFileChanged + m_bLinkTempFileChanged = false; + } + return; } @@ -1407,8 +1444,8 @@ void SAL_CALL OCommonEmbeddedObject::saveCompleted( sal_Bool bUseNew ) // for now support of this interface is required to allow breaking of links and converting them to normal embedded // objects, so the persist name must be handled correctly ( althowgh no real persist entry is used ) - // OSL_ENSURE( !m_bIsLink, "This method implementation must not be used for links!" ); - if ( m_bIsLink ) + // OSL_ENSURE( !m_bIsLinkURL, "This method implementation must not be used for links!" ); + if ( m_bIsLinkURL ) { if ( bUseNew ) m_aEntryName = m_aNewEntryName; @@ -1542,7 +1579,7 @@ void SAL_CALL OCommonEmbeddedObject::storeOwn() if ( !m_xDocHolder->GetComponent().is() ) throw uno::RuntimeException(); - if ( m_bIsLink ) + if ( m_bIsLinkURL ) { // TODO: just store the document to its location uno::Reference< frame::XStorable > xStorable( m_xDocHolder->GetComponent(), uno::UNO_QUERY_THROW ); @@ -1650,7 +1687,7 @@ void SAL_CALL OCommonEmbeddedObject::reload( "The object waits for saveCompleted() call!", static_cast< ::cppu::OWeakObject* >(this) ); - if ( m_bIsLink ) + if ( m_bIsLinkURL ) { // reload of the link OUString aOldLinkFilter = m_aLinkFilterName; @@ -1719,7 +1756,7 @@ void SAL_CALL OCommonEmbeddedObject::reload( if ( prop.Name == "ReadOnly" ) prop.Value >>= m_bReadOnly; - if ( bOldReadOnlyValue == m_bReadOnly || m_bIsLink ) + if ( bOldReadOnlyValue == m_bReadOnly || m_bIsLinkURL ) return; // close own storage @@ -1751,7 +1788,7 @@ void SAL_CALL OCommonEmbeddedObject::breakLink( const uno::Reference< embed::XSt if ( m_bDisposed ) throw lang::DisposedException(); // TODO - if (!m_bIsLink || m_nObjectState == -1) + if (!m_bIsLinkURL || m_nObjectState == -1) { // it must be a linked initialized object throw embed::WrongStateException( @@ -1820,7 +1857,10 @@ void SAL_CALL OCommonEmbeddedObject::breakLink( const uno::Reference< embed::XSt else if ( m_nObjectState == embed::EmbedStates::ACTIVE ) m_xDocHolder->Show(); - m_bIsLink = false; + // tdf#141529 reset all stuff involved in linked state, including + // the OLE content copied to the temp file + m_bIsLinkURL = false; + m_aLinkTempFile.clear(); m_aLinkFilterName.clear(); m_aLinkURL.clear(); } @@ -1832,7 +1872,7 @@ sal_Bool SAL_CALL OCommonEmbeddedObject::isLink() if ( m_bDisposed ) throw lang::DisposedException(); // TODO - return m_bIsLink; + return m_bIsLinkURL; } @@ -1842,7 +1882,7 @@ OUString SAL_CALL OCommonEmbeddedObject::getLinkURL() if ( m_bDisposed ) throw lang::DisposedException(); // TODO - if ( !m_bIsLink ) + if ( !m_bIsLinkURL ) throw embed::WrongStateException( "The object is not a link object!", static_cast< ::cppu::OWeakObject* >(this) ); diff --git a/embeddedobj/source/commonembedding/visobj.cxx b/embeddedobj/source/commonembedding/visobj.cxx index 6306ecfad0cf..851457609e7d 100644 --- a/embeddedobj/source/commonembedding/visobj.cxx +++ b/embeddedobj/source/commonembedding/visobj.cxx @@ -55,7 +55,7 @@ void SAL_CALL OCommonEmbeddedObject::setVisualAreaSize( sal_Int64 nAspect, const changeState( embed::EmbedStates::RUNNING ); // the links should be switched back to loaded state for now to avoid locking problems - bBackToLoaded = m_bIsLink; + bBackToLoaded = m_bIsLinkURL; } bool bSuccess = m_xDocHolder->SetExtent( nAspect, aSize ); @@ -88,7 +88,7 @@ awt::Size SAL_CALL OCommonEmbeddedObject::getVisualAreaSize( sal_Int64 nAspect ) changeState( embed::EmbedStates::RUNNING ); // the links should be switched back to loaded state for now to avoid locking problems - bBackToLoaded = m_bIsLink; + bBackToLoaded = m_bIsLinkURL; } awt::Size aResult; @@ -128,7 +128,7 @@ sal_Int32 SAL_CALL OCommonEmbeddedObject::getMapUnit( sal_Int64 nAspect ) changeState( embed::EmbedStates::RUNNING ); // the links should be switched back to loaded state for now to avoid locking problems - bBackToLoaded = m_bIsLink; + bBackToLoaded = m_bIsLinkURL; } sal_Int32 nResult = m_xDocHolder->GetMapUnit( nAspect ); @@ -172,7 +172,7 @@ embed::VisualRepresentation SAL_CALL OCommonEmbeddedObject::getPreferredVisualRe setVisualAreaSize(nAspect, aOrigSize); // the links should be switched back to loaded state for now to avoid locking problems - bBackToLoaded = m_bIsLink; + bBackToLoaded = m_bIsLinkURL; } SAL_WARN_IF( !m_xDocHolder->GetComponent().is(), "embeddedobj.common", "Running or Active object has no component!" ); diff --git a/embeddedobj/source/inc/commonembobj.hxx b/embeddedobj/source/inc/commonembobj.hxx index 2294b0d7e915..1b020f430855 100644 --- a/embeddedobj/source/inc/commonembobj.hxx +++ b/embeddedobj/source/inc/commonembobj.hxx @@ -32,6 +32,7 @@ #include <com/sun/star/awt/Rectangle.hpp> #include <com/sun/star/util/XCloseable.hpp> #include <com/sun/star/chart2/XDefaultSizeTransmitter.hpp> +#include <com/sun/star/io/XTempFile.hpp> #include <cppuhelper/weak.hxx> #include <rtl/ref.hxx> #include <map> @@ -132,7 +133,8 @@ protected: css::awt::Rectangle m_aOwnRectangle; css::awt::Rectangle m_aClipRectangle; - bool m_bIsLink; + bool m_bIsLinkURL; + bool m_bLinkTempFileChanged; // embedded object related stuff OUString m_aEntryName; @@ -146,6 +148,9 @@ protected: bool m_bLinkHasPassword; OUString m_aLinkPassword; + // tdf#141529 hold a cc of a linked OLE + css::uno::Reference < css::io::XTempFile > m_aLinkTempFile; + css::uno::Reference< css::uno::XInterface > m_xParent; bool m_bHasClonedSize; // the object has cached size _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits