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

Miklos Vajna vmiklos at collabora.co.uk
Tue Jul 18 13:27:08 UTC 2017


 sw/qa/extras/ooxmlexport/data/tdf106132.docx          |binary
 sw/qa/extras/ooxmlexport/ooxmlexport9.cxx             |    7 ++
 writerfilter/inc/ooxml/OOXMLDocument.hxx              |    4 +
 writerfilter/source/ooxml/OOXMLDocumentImpl.cxx       |   20 +++++++-
 writerfilter/source/ooxml/OOXMLDocumentImpl.hxx       |    6 ++
 writerfilter/source/ooxml/OOXMLFastContextHandler.cxx |   45 ++++++++++--------
 writerfilter/source/ooxml/OOXMLFastContextHandler.hxx |    2 
 7 files changed, 63 insertions(+), 21 deletions(-)

New commits:
commit 2cd91d79b90316f6a21db64bd944cb828b345a78
Author: Miklos Vajna <vmiklos at collabora.co.uk>
Date:   Tue Jul 11 09:04:23 2017 +0200

    tdf#106132 DOCX import: fix handling of nested textbox margins
    
    drawingML shapes are independent, and having a separate parser for each
    shape is a good thing, so that when we have an XML fragment like this:
    
    <wps:wsp>
      <wps:txbx>
      ...
      </wps:txbx>
      <wps:bodyPr .../>
    </wps:wsp>
    
    where <wps:txbx> may contain nested <wps:wsp> tags, we apply the
    properties specified in <wps:bodyPr> to the correct shape.
    
    In contrast, doing the same for VML is not a good idea, see commit
    476316bfc9dd36c0613327c20822a193b5ca8d9b (do reuse shape context,
    2012-05-22).
    
    So conditionally (for DML) maintain a stack of shape parsers, this way
    the outer <wps:bodyPr> is parsed and the result is applied to the
    correct shape, at the end resulting in correct inner margin for the outer
    shape in the bugdoc.
    
    This requires moving the context setup code from
    the OOXMLFastContextHandlerShape ctor to setToken(), as only then we
    know if this shape context will be used for VML or DML purposes. This
    should be OK, given that
    writerfilter::ooxml::OOXMLFastHelper::createAndSetParentAndDefine()
    calls setToken() right after allocating the context.
    
    (cherry picked from commit 6f9f9491bdef676f969898bd653db9905d14f5e8)
    
    Change-Id: I8c0d2f49cac76589fe269230698b203b5ca6996c
    Reviewed-on: https://gerrit.libreoffice.org/39830
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Michael Stahl <mstahl at redhat.com>

diff --git a/sw/qa/extras/ooxmlexport/data/tdf106132.docx b/sw/qa/extras/ooxmlexport/data/tdf106132.docx
new file mode 100644
index 000000000000..e342898141b1
Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf106132.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
index 81692f745778..b2ab8fdea9ed 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
@@ -363,6 +363,13 @@ DECLARE_OOXMLEXPORT_TEST(testTdf103573, "tdf103573.docx")
     CPPUNIT_ASSERT_EQUAL_MESSAGE("Not centered horizontally relatively to right page border", text::RelOrientation::PAGE_RIGHT, nValue);
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf106132, "tdf106132.docx")
+{
+    uno::Reference<beans::XPropertySet> xShape(getShape(1), uno::UNO_QUERY);
+    // This was 250, <wps:bodyPr ... rIns="0" ...> was ignored for an outer shape.
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), getProperty<sal_Int32>(xShape, "TextRightDistance"));
+}
+
 DECLARE_OOXMLEXPORT_TEST(testBnc519228OddBreaks, "bnc519228_odd-breaksB.docx")
 {
     // Check that all the normal styles are not set as right-only, those should be only those used after odd page breaks.
diff --git a/writerfilter/inc/ooxml/OOXMLDocument.hxx b/writerfilter/inc/ooxml/OOXMLDocument.hxx
index c7241f946311..8f0a12d3189c 100644
--- a/writerfilter/inc/ooxml/OOXMLDocument.hxx
+++ b/writerfilter/inc/ooxml/OOXMLDocument.hxx
@@ -221,6 +221,10 @@ public:
     virtual const OUString & getTarget() const = 0;
     virtual css::uno::Reference<css::xml::sax::XFastShapeContextHandler> getShapeContext( ) = 0;
     virtual void setShapeContext( css::uno::Reference<css::xml::sax::XFastShapeContextHandler> xContext ) = 0;
+    /// Push context of drawingML shapes, so nested shapes are handled separately.
+    virtual void pushShapeContext() = 0;
+    /// Pop context of a previously pushed drawingML shape.
+    virtual void popShapeContext() = 0;
     virtual css::uno::Reference<css::xml::dom::XDocument> getThemeDom( ) = 0;
     virtual css::uno::Reference<css::xml::dom::XDocument> getGlossaryDocDom( ) = 0;
     virtual css::uno::Sequence<css::uno::Sequence< css::uno::Any> > getGlossaryDomList() = 0;
diff --git a/writerfilter/source/ooxml/OOXMLDocumentImpl.cxx b/writerfilter/source/ooxml/OOXMLDocumentImpl.cxx
index 089c0e2a8f26..abb7c522e586 100644
--- a/writerfilter/source/ooxml/OOXMLDocumentImpl.cxx
+++ b/writerfilter/source/ooxml/OOXMLDocumentImpl.cxx
@@ -66,6 +66,7 @@ OOXMLDocumentImpl::OOXMLDocumentImpl(OOXMLStream::Pointer_t const & pStream, con
     , m_rBaseURL(utl::MediaDescriptor(rDescriptor).getUnpackedValueOrDefault("DocumentBaseURL", OUString()))
     , maMediaDescriptor(rDescriptor)
 {
+    pushShapeContext();
 }
 
 OOXMLDocumentImpl::~OOXMLDocumentImpl()
@@ -909,12 +910,27 @@ const uno::Sequence<beans::PropertyValue>& OOXMLDocumentImpl::getMediaDescriptor
 
 void OOXMLDocumentImpl::setShapeContext( uno::Reference<xml::sax::XFastShapeContextHandler> xContext )
 {
-    mxShapeContext = xContext;
+    if (!maShapeContexts.empty())
+        maShapeContexts.top() = xContext;
 }
 
 uno::Reference<xml::sax::XFastShapeContextHandler> OOXMLDocumentImpl::getShapeContext( )
 {
-    return mxShapeContext;
+    if (!maShapeContexts.empty())
+        return maShapeContexts.top();
+    else
+        return uno::Reference<xml::sax::XFastShapeContextHandler>();
+}
+
+void OOXMLDocumentImpl::pushShapeContext()
+{
+    maShapeContexts.push(uno::Reference<xml::sax::XFastShapeContextHandler>());
+}
+
+void OOXMLDocumentImpl::popShapeContext()
+{
+    if (!maShapeContexts.empty())
+        maShapeContexts.pop();
 }
 
 uno::Reference<xml::dom::XDocument> OOXMLDocumentImpl::getThemeDom( )
diff --git a/writerfilter/source/ooxml/OOXMLDocumentImpl.hxx b/writerfilter/source/ooxml/OOXMLDocumentImpl.hxx
index 43bdeb651d78..17fde7c89079 100644
--- a/writerfilter/source/ooxml/OOXMLDocumentImpl.hxx
+++ b/writerfilter/source/ooxml/OOXMLDocumentImpl.hxx
@@ -27,6 +27,7 @@
 #include "OOXMLPropertySet.hxx"
 
 #include <vector>
+#include <stack>
 
 namespace writerfilter {
 namespace ooxml
@@ -43,7 +44,8 @@ class OOXMLDocumentImpl : public OOXMLDocument
     css::uno::Reference<css::drawing::XDrawPage> mxDrawPage;
     css::uno::Reference<css::xml::dom::XDocument> mxGlossaryDocDom;
     css::uno::Sequence < css::uno::Sequence< css::uno::Any > > mxGlossaryDomList;
-    css::uno::Reference<css::xml::sax::XFastShapeContextHandler> mxShapeContext;
+    /// Stack of shape contexts, 1 element for VML, 1 element / nesting level for drawingML.
+    std::stack< css::uno::Reference<css::xml::sax::XFastShapeContextHandler> > maShapeContexts;
     css::uno::Reference<css::xml::dom::XDocument> mxThemeDom;
     css::uno::Sequence<css::uno::Reference<css::xml::dom::XDocument> > mxCustomXmlDomList;
     css::uno::Sequence<css::uno::Reference<css::xml::dom::XDocument> > mxCustomXmlDomPropsList;
@@ -128,6 +130,8 @@ public:
     virtual const OUString & getTarget() const override;
     virtual css::uno::Reference<css::xml::sax::XFastShapeContextHandler> getShapeContext( ) override;
     virtual void setShapeContext( css::uno::Reference<css::xml::sax::XFastShapeContextHandler> xContext ) override;
+    void pushShapeContext() override;
+    void popShapeContext() override;
     virtual css::uno::Reference<css::xml::dom::XDocument> getThemeDom() override;
     virtual css::uno::Sequence<css::uno::Reference<css::xml::dom::XDocument> > getCustomXmlDomList() override;
     virtual css::uno::Sequence<css::uno::Reference<css::xml::dom::XDocument> > getCustomXmlDomPropsList() override;
diff --git a/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx b/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx
index 48f51b4df512..f10a633f6a64 100644
--- a/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx
+++ b/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx
@@ -1551,29 +1551,14 @@ void OOXMLFastContextHandlerTextTable::lcl_endFastElement
 OOXMLFastContextHandlerShape::OOXMLFastContextHandlerShape
 (OOXMLFastContextHandler * pContext)
 : OOXMLFastContextHandlerProperties(pContext), m_bShapeSent( false ),
-    m_bShapeStarted(false)
+    m_bShapeStarted(false), m_bShapeContextPushed(false)
 {
-    mrShapeContext.set( getDocument( )->getShapeContext( ) );
-    if ( !mrShapeContext.is( ) )
-    {
-        // Define the shape context for the whole document
-        mrShapeContext = css::xml::sax::FastShapeContextHandler::create(
-            getComponentContext());
-        getDocument()->setShapeContext( mrShapeContext );
-    }
-
-    mrShapeContext->setModel(getDocument()->getModel());
-    uno::Reference<document::XDocumentPropertiesSupplier> xDocSupplier(getDocument()->getModel(), uno::UNO_QUERY_THROW);
-    mrShapeContext->setDocumentProperties(xDocSupplier->getDocumentProperties());
-    mrShapeContext->setDrawPage(getDocument()->getDrawPage());
-    mrShapeContext->setMediaDescriptor(getDocument()->getMediaDescriptor());
-
-    mrShapeContext->setRelationFragmentPath
-        (mpParserState->getTarget());
 }
 
 OOXMLFastContextHandlerShape::~OOXMLFastContextHandlerShape()
 {
+    if (m_bShapeContextPushed)
+        getDocument()->popShapeContext();
 }
 
 void OOXMLFastContextHandlerShape::lcl_startFastElement
@@ -1599,6 +1584,30 @@ void SAL_CALL OOXMLFastContextHandlerShape::startUnknownElement
 
 void OOXMLFastContextHandlerShape::setToken(Token_t nToken)
 {
+    if (nToken == Token_t(NMSP_wps | XML_wsp) || nToken == Token_t(NMSP_dmlPicture | XML_pic))
+    {
+        // drawingML shapes are independent, <wps:bodyPr> is not parsed after
+        // shape contents without pushing/popping the stack.
+        m_bShapeContextPushed = true;
+        getDocument()->pushShapeContext();
+    }
+
+    mrShapeContext.set(getDocument()->getShapeContext());
+    if (!mrShapeContext.is())
+    {
+        // Define the shape context for the whole document
+        mrShapeContext = css::xml::sax::FastShapeContextHandler::create(getComponentContext());
+        getDocument()->setShapeContext(mrShapeContext);
+    }
+
+    mrShapeContext->setModel(getDocument()->getModel());
+    uno::Reference<document::XDocumentPropertiesSupplier> xDocSupplier(getDocument()->getModel(), uno::UNO_QUERY_THROW);
+    mrShapeContext->setDocumentProperties(xDocSupplier->getDocumentProperties());
+    mrShapeContext->setDrawPage(getDocument()->getDrawPage());
+    mrShapeContext->setMediaDescriptor(getDocument()->getMediaDescriptor());
+
+    mrShapeContext->setRelationFragmentPath(mpParserState->getTarget());
+
     OOXMLFastContextHandler::setToken(nToken);
 
     if (mrShapeContext.is())
diff --git a/writerfilter/source/ooxml/OOXMLFastContextHandler.hxx b/writerfilter/source/ooxml/OOXMLFastContextHandler.hxx
index ba0ee9a3d66b..396dafd2b136 100644
--- a/writerfilter/source/ooxml/OOXMLFastContextHandler.hxx
+++ b/writerfilter/source/ooxml/OOXMLFastContextHandler.hxx
@@ -427,6 +427,8 @@ class OOXMLFastContextHandlerShape: public OOXMLFastContextHandlerProperties
 private:
     bool m_bShapeSent;
     bool m_bShapeStarted;
+    /// Is it necessary to pop the stack in the dtor?
+    bool m_bShapeContextPushed;
 
 public:
     explicit OOXMLFastContextHandlerShape(OOXMLFastContextHandler * pContext);


More information about the Libreoffice-commits mailing list