[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-6.0' - sc/qa sc/source

[Mike Kaganski (via logerrit)]

 sc/qa/unit/data/ods/tdf124651_simplePivotTable.ods |binary
 sc/qa/unit/pivottable_filters_test.cxx             |   33 +++++++++++++++++----
 sc/source/filter/excel/xepivotxml.cxx              |    7 +++-
 3 files changed, 32 insertions(+), 8 deletions(-)

New commits:
commit 1a28b0a602bb3c10f75c3c6408cf9bc555020d34
Author:     Mike Kaganski <mike.kaganski at collabora.com>
AuthorDate: Wed Apr 10 15:18:07 2019 +0300
Commit:     Mike Kaganski <mike.kaganski at collabora.com>
CommitDate: Fri Apr 12 23:09:26 2019 +0200

    tdf#124651: always write "name" attribute to dataField element
    
    Despite being optional as per ECMA-376-1:2016, Excel 2016 seems to
    require the presence of "name" attribute in dataField element of
    pivot table definition, so make sure to write at least empty string
    there.
    
    Change-Id: Iaab5674f86b7dd0b267776678e11af47086635d7
    Reviewed-on: https://gerrit.libreoffice.org/70522
    Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>
    Tested-by: Mike Kaganski <mike.kaganski at collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/70684

diff --git a/sc/qa/unit/data/ods/tdf124651_simplePivotTable.ods b/sc/qa/unit/data/ods/tdf124651_simplePivotTable.ods
new file mode 100644
index 000000000000..05fd5fec838f
Binary files /dev/null and b/sc/qa/unit/data/ods/tdf124651_simplePivotTable.ods differ
diff --git a/sc/qa/unit/pivottable_filters_test.cxx b/sc/qa/unit/pivottable_filters_test.cxx
index 9caf4d6a61b1..34b26d9a3e25 100644
--- a/sc/qa/unit/pivottable_filters_test.cxx
+++ b/sc/qa/unit/pivottable_filters_test.cxx
@@ -89,6 +89,7 @@ public:
     void testTdf112106();
     void testTdf123923();
     void testTdf123939();
+    void testTdf124651();
 
     CPPUNIT_TEST_SUITE(ScPivotTableFiltersTest);
 
@@ -132,6 +133,7 @@ public:
     CPPUNIT_TEST(testTdf112106);
     CPPUNIT_TEST(testTdf123923);
     CPPUNIT_TEST(testTdf123939);
+    CPPUNIT_TEST(testTdf124651);
 
     CPPUNIT_TEST_SUITE_END();
 
@@ -2355,17 +2357,21 @@ void ScPivotTableFiltersTest::testPivotTableDuplicateFields()
 
     std::shared_ptr<utl::TempFile> pXPathFile
         = ScBootstrapFixture::exportTo(&(*xShell), FORMAT_XLSX);
-    xmlDocPtr pCacheDef
-        = XPathHelper::parseExport(pXPathFile, m_xSFactory, "xl/pivotCache/pivotCacheDefinition1.xml");
+    xmlDocPtr pCacheDef = XPathHelper::parseExport(pXPathFile, m_xSFactory,
+                                                   "xl/pivotCache/pivotCacheDefinition1.xml");
     CPPUNIT_ASSERT(pCacheDef);
 
     assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields", "count", "6");
     assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]", "name", "ID");
     assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]", "name", "Name");
-    assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]", "name", "Score");
-    assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]", "name", "Method");
-    assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[5]", "name", "method2");
-    assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[6]", "name", "Method3");
+    assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]", "name",
+                "Score");
+    assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]", "name",
+                "Method");
+    assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[5]", "name",
+                "method2");
+    assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[6]", "name",
+                "Method3");
 
     xShell->DoClose();
 }
@@ -2443,6 +2449,21 @@ void ScPivotTableFiltersTest::testTdf123939()
                 "containsMixedTypes", "1");
 }
 
+void ScPivotTableFiltersTest::testTdf124651()
+{
+    ScDocShellRef xDocSh = loadDoc("tdf124651_simplePivotTable.", FORMAT_ODS);
+    CPPUNIT_ASSERT(xDocSh.is());
+
+    std::shared_ptr<utl::TempFile> pXPathFile
+        = ScBootstrapFixture::exportTo(&(*xDocSh), FORMAT_XLSX);
+    xmlDocPtr pDoc
+        = XPathHelper::parseExport(pXPathFile, m_xSFactory, "xl/pivotTables/pivotTable1.xml");
+    CPPUNIT_ASSERT(pDoc);
+    // We have to export name attribute, even though it's optional according to ECMA-376 standard,
+    // because Excel (at least 2016) seems to require it.
+    assertXPath(pDoc, "/x:pivotTableDefinition/x:dataFields/x:dataField", "name", "");
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(ScPivotTableFiltersTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sc/source/filter/excel/xepivotxml.cxx b/sc/source/filter/excel/xepivotxml.cxx
index 0594601b6eb3..d879f9c80a20 100644
--- a/sc/source/filter/excel/xepivotxml.cxx
+++ b/sc/source/filter/excel/xepivotxml.cxx
@@ -970,9 +970,12 @@ void XclExpXmlPivotTables::SavePivotTableXml( XclExpXmlStream& rStrm, const ScDP
             assert(aCachedDims[nDimIdx]); // the loop above should have screened for NULL's.
             const ScDPSaveDimension& rDim = *it->mpDim;
             const OUString* pName = rDim.GetLayoutName();
+            // tdf#124651: despite being optional in CT_DataField according to ECMA-376 Part 1,
+            // Excel (at least 2016) seems to insist on the presence of "name" attribute in
+            // dataField element, even if empty
+            const OString sName = pName ? XclXmlUtils::ToOString(*pName) : "";
             pPivotStrm->write("<")->writeId(XML_dataField);
-            if (pName)
-                rStrm.WriteAttributes(XML_name, XclXmlUtils::ToOString(*pName), FSEND);
+            rStrm.WriteAttributes(XML_name, sName, FSEND);
 
             rStrm.WriteAttributes(XML_fld, OString::number(nDimIdx).getStr(), FSEND);