[Libreoffice-commits] core.git: Branch 'libreoffice-5-2' - sw/qa sw/source

Miklos Vajna vmiklos at collabora.co.uk
Sun Oct 23 10:34:11 UTC 2016


 sw/qa/extras/ooxmlexport/data/tdf103001.docx |binary
 sw/qa/extras/ooxmlexport/ooxmlexport4.cxx    |   13 ++++++++++
 sw/source/filter/ww8/docxattributeoutput.cxx |   32 +++++++++++++++++++--------
 sw/source/filter/ww8/docxattributeoutput.hxx |    7 +++--
 sw/source/filter/ww8/docxexport.cxx          |    8 +++---
 5 files changed, 44 insertions(+), 16 deletions(-)

New commits:
commit 3f373500282c926031eed4f995ca8d51402ed187
Author: Miklos Vajna <vmiklos at collabora.co.uk>
Date:   Tue Oct 11 08:03:58 2016 +0200

    tdf#103001 DOCX export: fix RelId cache when switching streams
    
    RelIds are used to refer to external streams, e.g. images. A RelId cache
    tries to avoid storing the same image more than once in the export
    result. RelIds are relative to an XML stream, so caching them across
    stream switches is problematic. There was code already to notify the
    cache after a header or footer was written, but:
    
    1) It was only done after a switch, not before and
    
    2) It cleared the whole cache, instead of stashing it away, and
    restoring it when the write of the special stream is done.
    
    Regression from commit b484e9814c66d8d51cea974390963a6944bc9d73
    (tdf#83227 oox: reuse RelId in DML/VML export for the same graphic,
    2015-09-07), this existing problem became more visible when caching
    started to include draw images, not just writer ones.
    
    Fix both problems by turning the cache into a stack that is
    pushed/popped around stream changes.
    
    (cherry picked from commit 3906d25a2e12123aee54654ad26699a2832389d4)
    
    Change-Id: If9ec168823eea5e272197e28f6125ba626605550
    Reviewed-on: https://gerrit.libreoffice.org/29832
    Reviewed-by: Tamás Zolnai <zolnaitamas2000 at gmail.com>
    Tested-by: Tamás Zolnai <zolnaitamas2000 at gmail.com>

diff --git a/sw/qa/extras/ooxmlexport/data/tdf103001.docx b/sw/qa/extras/ooxmlexport/data/tdf103001.docx
new file mode 100644
index 0000000..332d415
Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf103001.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx
index f763598..c199d6f 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx
@@ -952,6 +952,19 @@ DECLARE_OOXMLEXPORT_TEST(testTdf83227, "tdf83227.docx")
     CPPUNIT_ASSERT_EQUAL(false, bool(xNameAccess->hasByName("word/media/image2.png")));
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf103001, "tdf103001.docx")
+{
+    // The same image is featured in the header and in the body text, make sure
+    // the header relation is still written, even when caching is enabled.
+    if (!mbExported)
+        return;
+
+    uno::Reference<packages::zip::XZipFileAccess2> xNameAccess = packages::zip::ZipFileAccess::createWithURL(comphelper::getComponentContext(m_xSFactory), maTempFile.GetURL());
+    // This failed: header reused the RelId of the body text, even if RelIds
+    // are local to their stream.
+    CPPUNIT_ASSERT(xNameAccess->hasByName("word/_rels/header1.xml.rels"));
+}
+
 DECLARE_OOXMLEXPORT_TEST(testTdf92521, "tdf92521.odt")
 {
     if (xmlDocPtr pXmlDoc = parseExport("word/document.xml"))
diff --git a/sw/source/filter/ww8/docxattributeoutput.cxx b/sw/source/filter/ww8/docxattributeoutput.cxx
index 9c1d832..da5f181 100644
--- a/sw/source/filter/ww8/docxattributeoutput.cxx
+++ b/sw/source/filter/ww8/docxattributeoutput.cxx
@@ -4170,25 +4170,34 @@ void DocxAttributeOutput::WriteSrcRect(const SdrObject* pSdrObj )
     }
 }
 
-void DocxAttributeOutput::ClearRelIdCache()
+void DocxAttributeOutput::PopRelIdCache()
 {
-    m_aRelIdCache.clear();
-    m_aSdrRelIdCache.clear();
+    if (!m_aRelIdCache.empty())
+        m_aRelIdCache.pop();
+    if (!m_aSdrRelIdCache.empty())
+        m_aSdrRelIdCache.pop();
+}
+
+void DocxAttributeOutput::PushRelIdCache()
+{
+    m_aRelIdCache.push(std::map<const Graphic*, OString>());
+    m_aSdrRelIdCache.push(std::map<BitmapChecksum, OUString>());
 }
 
 OUString DocxAttributeOutput::FindRelId(BitmapChecksum nChecksum)
 {
     OUString aRet;
 
-    if (m_aSdrRelIdCache.find(nChecksum) != m_aSdrRelIdCache.end())
-        aRet = m_aSdrRelIdCache[nChecksum];
+    if (!m_aSdrRelIdCache.empty() && m_aSdrRelIdCache.top().find(nChecksum) != m_aSdrRelIdCache.top().end())
+        aRet = m_aSdrRelIdCache.top()[nChecksum];
 
     return aRet;
 }
 
 void DocxAttributeOutput::CacheRelId(BitmapChecksum nChecksum, const OUString& rRelId)
 {
-     m_aSdrRelIdCache[nChecksum] = rRelId;
+    if (!m_aSdrRelIdCache.empty())
+        m_aSdrRelIdCache.top()[nChecksum] = rRelId;
 }
 
 void DocxAttributeOutput::FlyFrameGraphic( const SwGrfNode* pGrfNode, const Size& rSize, const SwFlyFrameFormat* pOLEFrameFormat, SwOLENode* pOLENode, const SdrObject* pSdrObj )
@@ -4226,9 +4235,9 @@ void DocxAttributeOutput::FlyFrameGraphic( const SwGrfNode* pGrfNode, const Size
         else
             pGraphic = pOLENode->GetGraphic();
 
-        if (m_aRelIdCache.find(pGraphic) != m_aRelIdCache.end())
+        if (!m_aRelIdCache.empty() && m_aRelIdCache.top().find(pGraphic) != m_aRelIdCache.top().end())
             // We already have a RelId for this Graphic.
-            aRelId = m_aRelIdCache[pGraphic];
+            aRelId = m_aRelIdCache.top()[pGraphic];
         else
         {
             // Not in cache, then need to write it.
@@ -4237,7 +4246,8 @@ void DocxAttributeOutput::FlyFrameGraphic( const SwGrfNode* pGrfNode, const Size
             OUString aImageId = m_rDrawingML.WriteImage( *pGraphic );
 
             aRelId = OUStringToOString( aImageId, RTL_TEXTENCODING_UTF8 );
-            m_aRelIdCache[pGraphic] = aRelId;
+            if (!m_aRelIdCache.empty())
+                m_aRelIdCache.top()[pGraphic] = aRelId;
         }
 
         nImageType = XML_embed;
@@ -8503,6 +8513,10 @@ DocxAttributeOutput::DocxAttributeOutput( DocxExport &rExport, FSHelperPtr pSeri
     , m_nStateOfFlyFrame( FLY_NOT_PROCESSED )
     , m_bParagraphSdtHasId(false)
 {
+    // Push initial items to the RelId cache. In case the document contains no
+    // special streams (headers, footers, etc.) then these items are used
+    // during the full export.
+    PushRelIdCache();
 }
 
 DocxAttributeOutput::~DocxAttributeOutput()
diff --git a/sw/source/filter/ww8/docxattributeoutput.hxx b/sw/source/filter/ww8/docxattributeoutput.hxx
index 41f9189..701bfc6 100644
--- a/sw/source/filter/ww8/docxattributeoutput.hxx
+++ b/sw/source/filter/ww8/docxattributeoutput.hxx
@@ -371,7 +371,8 @@ public:
 
     void WriteBookmarks_Impl( std::vector< OUString >& rStarts, std::vector< OUString >& rEnds );
     void WriteAnnotationMarks_Impl( std::vector< OUString >& rStarts, std::vector< OUString >& rEnds );
-    void ClearRelIdCache();
+    void PushRelIdCache();
+    void PopRelIdCache();
     /// End possibly opened paragraph sdt block.
     void EndParaSdtBlock();
 
@@ -915,9 +916,9 @@ private:
     bool m_setFootnote;
 
     /// RelId <-> Graphic* cache, so that in case of alternate content, the same graphic only gets written once.
-    std::map<const Graphic*, OString> m_aRelIdCache;
+    std::stack< std::map<const Graphic*, OString> > m_aRelIdCache;
     /// RelId <-> BitmapChecksum cache, similar to m_aRelIdCache, but used for non-Writer graphics, handled in oox.
-    std::map<BitmapChecksum, OUString> m_aSdrRelIdCache;
+    std::stack< std::map<BitmapChecksum, OUString> > m_aSdrRelIdCache;
 
     /// members to control the existence of grabbagged SDT properties in the paragraph
     sal_Int32 m_nParagraphSdtPrToken;
diff --git a/sw/source/filter/ww8/docxexport.cxx b/sw/source/filter/ww8/docxexport.cxx
index 6ae3f99..4290098 100644
--- a/sw/source/filter/ww8/docxexport.cxx
+++ b/sw/source/filter/ww8/docxexport.cxx
@@ -724,15 +724,15 @@ void DocxExport::WriteHeaderFooter( const SwFormat& rFormat, bool bHeader, const
 
     DocxTableExportContext aTableExportContext;
     m_pAttrOutput->pushToTableExportContext(aTableExportContext);
+    //When the stream changes the cache which is maintained for the graphics in case of alternate content is not cleared.
+    //So clearing the alternate content graphic cache.
+    m_pAttrOutput->PushRelIdCache();
     // do the work
     WriteHeaderFooterText( rFormat, bHeader );
+    m_pAttrOutput->PopRelIdCache();
     m_pAttrOutput->popFromTableExportContext(aTableExportContext);
     m_pAttrOutput->EndParaSdtBlock();
 
-    //When the stream changes the cache which is maintained for the graphics in case of alternate content is not cleared.
-    //So clearing the alternate content graphic cache.
-    m_pAttrOutput->ClearRelIdCache();
-
     // switch the serializer back
     m_pAttrOutput->SetSerializer( m_pDocumentFS );
     m_pVMLExport->SetFS( m_pDocumentFS );


More information about the Libreoffice-commits mailing list