[Libreoffice-commits] core.git: Branch 'libreoffice-6-1' - sw/qa test/source writerfilter/source

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Tue Oct 30 17:34:15 UTC 2018


 sw/qa/extras/layout/data/tdf116989.docx                  |binary
 sw/qa/extras/layout/layout.cxx                           |   24 +++++++++
 test/source/xmltesttools.cxx                             |   39 ++++++++++++---
 writerfilter/source/dmapper/DomainMapperTableHandler.cxx |    3 -
 writerfilter/source/dmapper/DomainMapper_Impl.cxx        |   17 +++---
 writerfilter/source/dmapper/DomainMapper_Impl.hxx        |   10 +++
 6 files changed, 75 insertions(+), 18 deletions(-)

New commits:
commit 7df33caac85ac90c26e97dedbc201f46dc9e4cb4
Author:     Mike Kaganski <mike.kaganski at collabora.com>
AuthorDate: Tue Oct 30 17:17:46 2018 +0300
Commit:     Miklos Vajna <vmiklos at collabora.co.uk>
CommitDate: Tue Oct 30 18:33:51 2018 +0100

    tdf#116989: disable conversion of tables in footers to floating for now
    
    As the floating objects anchored to footers aren't wrapped around properly
    (they behave as if they are wrapped through unconditionally), which makes
    imported tables to overlap the page body text making the document unusable,
    let's just disable the conversion for the time being (until the actual bug
    fixed properly).
    
    This also backports the unit test from https://gerrit.libreoffice.org/62544
    (Change-Id Ia8b5478b0d2a15f91add4ee76455e73c2c970392).
    
    Change-Id: I06c984ff7157b71fff2aa8122cc475a1199994c6
    Reviewed-on: https://gerrit.libreoffice.org/62523
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/62628
    Reviewed-by: Miklos Vajna <vmiklos at collabora.co.uk>

diff --git a/sw/qa/extras/layout/data/tdf116989.docx b/sw/qa/extras/layout/data/tdf116989.docx
new file mode 100644
index 000000000000..498b60dbf345
Binary files /dev/null and b/sw/qa/extras/layout/data/tdf116989.docx differ
diff --git a/sw/qa/extras/layout/layout.cxx b/sw/qa/extras/layout/layout.cxx
index 21b682e1fb2f..0e2bc919f75e 100755
--- a/sw/qa/extras/layout/layout.cxx
+++ b/sw/qa/extras/layout/layout.cxx
@@ -30,6 +30,7 @@ public:
     void testTdf118058();
     void testTdf117188();
     void testTdf119875();
+    void testTdf116989();
 
     CPPUNIT_TEST_SUITE(SwLayoutWriter);
     CPPUNIT_TEST(testTdf116830);
@@ -45,6 +46,7 @@ public:
     CPPUNIT_TEST(testTdf118058);
     CPPUNIT_TEST(testTdf117188);
     CPPUNIT_TEST(testTdf119875);
+    CPPUNIT_TEST(testTdf116989);
     CPPUNIT_TEST_SUITE_END();
 
 private:
@@ -275,6 +277,28 @@ void SwLayoutWriter::testTdf119875()
     CPPUNIT_ASSERT_LESS(nSecondTop, nFirstTop);
 }
 
+void SwLayoutWriter::testTdf116989()
+{
+    createDoc("tdf116989.docx");
+    xmlDocPtr pXmlDoc = parseLayoutDump();
+    // FIXME: the XPath should be adjusted when the proper floating table would be imported
+    const sal_Int32 nTblTop
+        = getXPath(pXmlDoc, "/root/page[1]/footer/tab/infos/bounds", "top").toInt32();
+    const sal_Int32 nFirstPageParaCount
+        = getXPathContent(pXmlDoc, "count(/root/page[1]/body/txt)").toInt32();
+    // FIXME: should be exactly 30, when proper floating tables in footers are supported
+    CPPUNIT_ASSERT_GREATEREQUAL(sal_Int32(30), nFirstPageParaCount);
+    for (sal_Int32 i = 1; i <= nFirstPageParaCount; ++i)
+    {
+        const OString xPath = "/root/page[1]/body/txt[" + OString::number(i) + "]/infos/bounds";
+        const sal_Int32 nTxtBottom = getXPath(pXmlDoc, xPath.getStr(), "top").toInt32()
+                                     + getXPath(pXmlDoc, xPath.getStr(), "height").toInt32();
+        // No body paragraphs should overlap the table in the footer
+        CPPUNIT_ASSERT_MESSAGE(OString("testing paragraph #" + OString::number(i)).getStr(),
+                               nTxtBottom <= nTblTop);
+    }
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(SwLayoutWriter);
 CPPUNIT_PLUGIN_IMPLEMENT();
 
diff --git a/test/source/xmltesttools.cxx b/test/source/xmltesttools.cxx
index 45347b0c111b..ab373ccae96c 100644
--- a/test/source/xmltesttools.cxx
+++ b/test/source/xmltesttools.cxx
@@ -82,15 +82,40 @@ OUString XmlTestTools::getXPath(xmlDocPtr pXmlDoc, const OString& rXPath, const
 OUString XmlTestTools::getXPathContent(xmlDocPtr pXmlDoc, const OString& rXPath)
 {
     xmlXPathObjectPtr pXmlObj = getXPathNode(pXmlDoc, rXPath);
-    xmlNodeSetPtr pXmlNodes = pXmlObj->nodesetval;
+    switch (pXmlObj->type)
+    {
+        case XPATH_UNDEFINED:
+            CPPUNIT_FAIL("Undefined XPath type");
+        case XPATH_NODESET:
+        {
+            xmlNodeSetPtr pXmlNodes = pXmlObj->nodesetval;
 
-    CPPUNIT_ASSERT_MESSAGE(OString("In <" + OString(pXmlDoc->name) + ">, XPath '" + rXPath + "' not found").getStr(),
-            xmlXPathNodeSetGetLength(pXmlNodes) > 0);
+            CPPUNIT_ASSERT_MESSAGE(
+                OString("In <" + OString(pXmlDoc->name) + ">, XPath '" + rXPath + "' not found")
+                    .getStr(),
+                xmlXPathNodeSetGetLength(pXmlNodes) > 0);
 
-    xmlNodePtr pXmlNode = pXmlNodes->nodeTab[0];
-    OUString s(convert((pXmlNode->children[0]).content));
-    xmlXPathFreeObject(pXmlObj);
-    return s;
+            xmlNodePtr pXmlNode = pXmlNodes->nodeTab[0];
+            OUString s(convert(pXmlNode->children[0].content));
+            xmlXPathFreeObject(pXmlObj);
+            return s;
+        }
+        case XPATH_BOOLEAN:
+            return pXmlObj->boolval ? OUString("true") : OUString("false");
+        case XPATH_NUMBER:
+            return OUString::number(pXmlObj->floatval);
+        case XPATH_STRING:
+            return convert(pXmlObj->stringval);
+        case XPATH_POINT:
+        case XPATH_RANGE:
+        case XPATH_LOCATIONSET:
+        case XPATH_USERS:
+        case XPATH_XSLT_TREE:
+            CPPUNIT_FAIL("Unsupported XPath type");
+    }
+
+    CPPUNIT_FAIL("Invalid XPath type");
+    return OUString(); // to suppress "Not all control paths return a value" warning on MSVC
 }
 
 void XmlTestTools::assertXPath(xmlDocPtr pXmlDoc, const OString& rXPath, const OString& rAttribute, const OUString& rExpectedValue)
diff --git a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
index e8f44771b030..89bc39979770 100644
--- a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
+++ b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
@@ -1191,7 +1191,8 @@ void DomainMapperTableHandler::endTable(unsigned int nestedTableLevel, bool bTab
             m_aTableProperties->getValue(TablePropertyMap::TABLE_WIDTH_TYPE, nTableWidthType);
             if (m_rDMapper_Impl.GetSectionContext() && nestedTableLevel <= 1 && !m_rDMapper_Impl.IsInHeaderFooter())
                 m_rDMapper_Impl.m_aPendingFloatingTables.emplace_back(xStart, xEnd, comphelper::containerToSequence(aFrameProperties), nTableWidth, nTableWidthType);
-            else
+            else if (!m_rDMapper_Impl.IsInFooter()) // FIXME: tdf#116989 floating objects anchored
+                                                    //to footer cannot have proper wrapping
             {
                 // m_xText points to the body text, get the current xText from m_rDMapper_Impl, in case e.g. we would be in a header.
                 uno::Reference<text::XTextAppendAndConvert> xTextAppendAndConvert(m_rDMapper_Impl.GetTopTextAppend(), uno::UNO_QUERY);
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 444b19c6de6c..bfdcad2effb8 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -210,7 +210,7 @@ DomainMapper_Impl::DomainMapper_Impl(
         m_sCurrentParaStyleName(),
         m_bInStyleSheetImport( false ),
         m_bInAnyTableImport( false ),
-        m_bInHeaderFooterImport( false ),
+        m_eInHeaderFooterImport( HeaderFooterImportState::none ),
         m_bDiscardHeaderFooter( false ),
         m_bInFootOrEndnote(false),
         m_bSeenFootOrEndnoteSeparator(false),
@@ -420,7 +420,7 @@ void DomainMapper_Impl::RemoveLastParagraph( )
         // (but only for paste/insert, not load; otherwise it can happen that
         // flys anchored at the disposed paragraph are deleted (fdo47036.rtf))
         bool const bEndOfDocument(m_aTextAppendStack.size() == 1);
-        if ((m_bInHeaderFooterImport || (bEndOfDocument && !m_bIsNewDoc))
+        if ((IsInHeaderFooter() || (bEndOfDocument && !m_bIsNewDoc))
             && xEnumerationAccess.is())
         {
             uno::Reference<container::XEnumeration> xEnumeration = xEnumerationAccess->createEnumeration();
@@ -1468,7 +1468,7 @@ void DomainMapper_Impl::appendTextPortion( const OUString& rString, const Proper
             {
                 if (m_bStartTOC || m_bStartIndex || m_bStartBibliography || m_bStartGenericField)
                 {
-                    if(m_bInHeaderFooterImport && !m_bStartTOCHeaderFooter)
+                    if (IsInHeaderFooter() && !m_bStartTOCHeaderFooter)
                     {
                         xTextRange = xTextAppend->appendTextPortion(rString, aValues);
                     }
@@ -1732,7 +1732,8 @@ void DomainMapper_Impl::PushPageHeaderFooter(bool bHeader, SectionPropertyMap::P
     const PropertyIds ePropTextLeft = bHeader? PROP_HEADER_TEXT_LEFT: PROP_FOOTER_TEXT_LEFT;
     const PropertyIds ePropText = bHeader? PROP_HEADER_TEXT: PROP_FOOTER_TEXT;
 
-    m_bInHeaderFooterImport = true;
+    m_eInHeaderFooterImport
+        = bHeader ? HeaderFooterImportState::header : HeaderFooterImportState::footer;
 
     //get the section context
     PropertyMapPtr pContext = DomainMapper_Impl::GetTopContextOfType(CONTEXT_SECTION);
@@ -1809,7 +1810,7 @@ void DomainMapper_Impl::PopPageHeaderFooter()
         }
         m_bDiscardHeaderFooter = false;
     }
-    m_bInHeaderFooterImport = false;
+    m_eInHeaderFooterImport = HeaderFooterImportState::none;
 
     if (!m_aHeaderFooterStack.empty())
     {
@@ -2218,7 +2219,7 @@ void DomainMapper_Impl::PushShapeContext( const uno::Reference< drawing::XShape
                     }
                 }
             }
-            if (!m_bInHeaderFooterImport && !checkZOrderStatus)
+            if (!IsInHeaderFooter() && !checkZOrderStatus)
                 xProps->setPropertyValue(
                         getPropertyName( PROP_OPAQUE ),
                         uno::makeAny( true ) );
@@ -3483,7 +3484,7 @@ void DomainMapper_Impl::handleToc
 {
     OUString sValue;
     m_bStartTOC = true;
-    if(m_bInHeaderFooterImport)
+    if (IsInHeaderFooter())
         m_bStartTOCHeaderFooter = true;
     bool bTableOfFigures = false;
     bool bHyperlinks = false;
@@ -4817,7 +4818,7 @@ void DomainMapper_Impl::PopFieldContext()
                     m_bStartTOC = false;
                     m_bStartIndex = false;
                     m_bStartBibliography = false;
-                    if(m_bInHeaderFooterImport && m_bStartTOCHeaderFooter)
+                    if (IsInHeaderFooter() && m_bStartTOCHeaderFooter)
                         m_bStartTOCHeaderFooter = false;
                 }
                 else
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index dc229c28b614..cc014db53a00 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -477,7 +477,12 @@ private:
     OUString                        m_sCurrentParaStyleName; //highly inaccurate. Overwritten by "overlapping" paragraphs like comments, flys.
     bool                            m_bInStyleSheetImport; //in import of fonts, styles, lists or lfos
     bool                            m_bInAnyTableImport; //in import of fonts, styles, lists or lfos
-    bool                            m_bInHeaderFooterImport;
+    enum class HeaderFooterImportState
+    {
+        none,
+        header,
+        footer,
+    }                               m_eInHeaderFooterImport;
     bool                            m_bDiscardHeaderFooter;
     bool                            m_bInFootOrEndnote;
     /// Did we get a <w:separator/> for this footnote already?
@@ -714,7 +719,8 @@ public:
     void PushPageFooter(SectionPropertyMap::PageType eType);
 
     void PopPageHeaderFooter();
-    bool IsInHeaderFooter() const { return m_bInHeaderFooterImport; }
+    bool IsInHeaderFooter() const { return m_eInHeaderFooterImport != HeaderFooterImportState::none; }
+    bool IsInFooter() const { return m_eInHeaderFooterImport == HeaderFooterImportState::footer; }
 
     bool IsInTOC() const { return m_bStartTOC; }
 


More information about the Libreoffice-commits mailing list