[Libreoffice-commits] core.git: Branch 'distro/lhm/libreoffice-6-4+backports' - sc/qa sc/source

Vasily Melenchuk (via logerrit) logerrit at kemper.freedesktop.org
Mon Oct 4 13:08:25 UTC 2021


 sc/qa/unit/data/xlsx/autofilter-colors-fg.xlsx |binary
 sc/qa/unit/subsequent_export-test.cxx          |   64 ++++++++++++++++---------
 sc/source/filter/excel/excrecds.cxx            |   17 ++++--
 sc/source/filter/excel/xestyle.cxx             |   22 +++-----
 sc/source/filter/inc/xestyle.hxx               |    6 --
 sc/source/filter/oox/autofilterbuffer.cxx      |   14 +----
 sc/source/filter/oox/stylesbuffer.cxx          |    7 +-
 7 files changed, 72 insertions(+), 58 deletions(-)

New commits:
commit dfdb58e7f40b6435e14ca9e4d10ba7b2b2a1ca3a
Author:     Vasily Melenchuk <vasily.melenchuk at cib.de>
AuthorDate: Fri Sep 24 15:18:13 2021 +0200
Commit:     Thorsten Behrens <thorsten.behrens at allotropia.de>
CommitDate: Mon Oct 4 15:07:50 2021 +0200

    tdf#143104 Fix xlsx import/export of color filter colors
    
    1. In XLSX filter colors are always stored in dxf as foreground
    colors, so Calc should keep them, if possible. So practically
    use only one color during import.
    
    3. On export we need to distinguish type of filter, this is done
    with cellColor=0 or cellColor=1 attribute of <colorFilter>.
    
    4. Since p.1 there is no need to keep on export separate dxf
    structures for fg and bg colors: we always use only foreground
    color for color filters.
    
    Co-authored-by: Samuel Mehrbrodt <samuel.mehrbrodt at allotropia.de>
    
    
    Change-Id: Iacd352ae46bf84859dc15ee695b6dc63240afe7d
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122593
    Tested-by: Jenkins
    Reviewed-by: Thorsten Behrens <thorsten.behrens at allotropia.de>
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122969
    Tested-by: Thorsten Behrens <thorsten.behrens at allotropia.de>

diff --git a/sc/qa/unit/data/xlsx/autofilter-colors-fg.xlsx b/sc/qa/unit/data/xlsx/autofilter-colors-fg.xlsx
new file mode 100644
index 000000000000..8360ec7e92be
Binary files /dev/null and b/sc/qa/unit/data/xlsx/autofilter-colors-fg.xlsx differ
diff --git a/sc/qa/unit/subsequent_export-test.cxx b/sc/qa/unit/subsequent_export-test.cxx
index bbfbbbf6e3e1..32ef315d440d 100644
--- a/sc/qa/unit/subsequent_export-test.cxx
+++ b/sc/qa/unit/subsequent_export-test.cxx
@@ -203,7 +203,6 @@ public:
     void testTdf95640_xlsx_to_xlsx();
     void testAutofilterColorsODF();
     void testAutofilterColorsOOXML();
-    void testAutofilterColorsStyleOOXML();
 
     void testRefStringXLSX();
     void testRefStringConfigXLSX();
@@ -342,7 +341,6 @@ public:
     CPPUNIT_TEST(testTdf95640_xlsx_to_xlsx);
     CPPUNIT_TEST(testAutofilterColorsODF);
     CPPUNIT_TEST(testAutofilterColorsOOXML);
-    CPPUNIT_TEST(testAutofilterColorsStyleOOXML);
 
     CPPUNIT_TEST(testRefStringXLSX);
     CPPUNIT_TEST(testRefStringConfigXLSX);
@@ -4101,27 +4099,49 @@ void ScExportTest::testAutofilterColorsODF()
 
 void ScExportTest::testAutofilterColorsOOXML()
 {
-    ScDocShellRef xDocSh = loadDoc(u"autofilter-colors.", FORMAT_XLSX);
-    CPPUNIT_ASSERT(xDocSh.is());
-
-    xmlDocPtr pDoc = XPathHelper::parseExport2(*this, *xDocSh, m_xSFactory,
-                                                     "xl/tables/table1.xml", FORMAT_XLSX);
-    CPPUNIT_ASSERT(pDoc);
-
-    assertXPath(pDoc, "/x:table/x:autoFilter/x:filterColumn/x:colorFilter", "dxfId", "5");
-}
-
-void ScExportTest::testAutofilterColorsStyleOOXML()
-{
-    ScDocShellRef xDocSh = loadDoc(u"autofilter-colors.", FORMAT_XLSX);
-    CPPUNIT_ASSERT(xDocSh.is());
-
-    xmlDocPtr pDoc
-        = XPathHelper::parseExport2(*this, *xDocSh, m_xSFactory, "xl/styles.xml", FORMAT_XLSX);
-    CPPUNIT_ASSERT(pDoc);
+    {
+        ScDocShellRef xDocSh = loadDoc(u"autofilter-colors.", FORMAT_XLSX);
+        CPPUNIT_ASSERT(xDocSh.is());
+        std::shared_ptr<utl::TempFile> pXPathFile
+            = ScBootstrapFixture::exportTo(&(*xDocSh), FORMAT_XLSX);
+        xmlDocPtr pTable1
+            = XPathHelper::parseExport(pXPathFile, m_xSFactory, "xl/tables/table1.xml");
+        CPPUNIT_ASSERT(pTable1);
+        sal_Int32 nDxfId
+            = getXPath(pTable1, "/x:table/x:autoFilter/x:filterColumn/x:colorFilter", "dxfId")
+                  .toInt32()
+              + 1;
+
+        xmlDocPtr pStyles
+            = XPathHelper::parseExport(pXPathFile, m_xSFactory, "xl/styles.xml");
+        CPPUNIT_ASSERT(pStyles);
+        OString sDxfXPath("/x:styleSheet/x:dxfs/x:dxf[" + OString::number(nDxfId)
+                          + "]/x:fill/x:patternFill/x:fgColor");
+        assertXPath(pStyles, sDxfXPath, "rgb", "FFFFD7D7");
+        xDocSh->DoClose();
+    }
 
-    assertXPath(pDoc, "/x:styleSheet/x:dxfs/x:dxf[5]/x:fill/x:patternFill/x:bgColor", "rgb",
-                "FFFFD7D7");
+    {
+        ScDocShellRef xDocSh = loadDoc(u"autofilter-colors-fg.", FORMAT_XLSX);
+        CPPUNIT_ASSERT(xDocSh.is());
+        std::shared_ptr<utl::TempFile> pXPathFile
+            = ScBootstrapFixture::exportTo(&(*xDocSh), FORMAT_XLSX);
+        xmlDocPtr pTable1
+            = XPathHelper::parseExport(pXPathFile, m_xSFactory, "xl/tables/table1.xml");
+        CPPUNIT_ASSERT(pTable1);
+        sal_Int32 nDxfId
+            = getXPath(pTable1, "/x:table/x:autoFilter/x:filterColumn/x:colorFilter", "dxfId")
+                  .toInt32()
+              + 1;
+
+        xmlDocPtr pStyles
+            = XPathHelper::parseExport(pXPathFile, m_xSFactory, "xl/styles.xml");
+        CPPUNIT_ASSERT(pStyles);
+        OString sDxfXPath("/x:styleSheet/x:dxfs/x:dxf[" + OString::number(nDxfId)
+                          + "]/x:fill/x:patternFill/x:fgColor");
+        assertXPath(pStyles, sDxfXPath, "rgb", "FF3465A4");
+        xDocSh->DoClose();
+    }
 }
 
 void ScExportTest::testConditionalFormatPriorityCheckXLSX()
diff --git a/sc/source/filter/excel/excrecds.cxx b/sc/source/filter/excel/excrecds.cxx
index 95bfeff9ef05..35a19585ba97 100644
--- a/sc/source/filter/excel/excrecds.cxx
+++ b/sc/source/filter/excel/excrecds.cxx
@@ -24,6 +24,7 @@
 
 #include <svl/zforlist.hxx>
 #include <sal/log.hxx>
+#include <sax/fastattribs.hxx>
 
 #include <string.h>
 
@@ -839,13 +840,19 @@ void XclExpAutofilter::SaveXml( XclExpXmlStream& rStrm )
             if (!maColorValues.empty())
             {
                 Color color = maColorValues[0].first;
-                sal_Int32 nDxfId;
+                sax_fastparser::FastAttributeList* pAttrList = sax_fastparser::FastSerializerHelper::createAttrList();
+
                 if (maColorValues[0].second) // is background color
-                    nDxfId = GetDxfs().GetDxfByBackColor(color);
+                {
+                    pAttrList->add(XML_cellColor, OString::number(1));
+                }
                 else
-                    nDxfId = GetDxfs().GetDxfByForeColor(color);
-                nDxfId++; // Count is 1-based
-                rWorksheet->singleElement(XML_colorFilter, XML_dxfId, OString::number(nDxfId));
+                {
+                    pAttrList->add(XML_cellColor, OString::number(0));
+                }
+                pAttrList->add(XML_dxfId, OString::number(GetDxfs().GetDxfByColor(color)));
+                sax_fastparser::XFastAttributeListRef xAttributeList(pAttrList);
+                rWorksheet->singleElement(XML_colorFilter, xAttributeList);
             }
         }
         break;
diff --git a/sc/source/filter/excel/xestyle.cxx b/sc/source/filter/excel/xestyle.cxx
index 017d8ad43dd1..c5ff879088c4 100644
--- a/sc/source/filter/excel/xestyle.cxx
+++ b/sc/source/filter/excel/xestyle.cxx
@@ -3057,18 +3057,20 @@ XclExpDxfs::XclExpDxfs( const XclExpRoot& rRoot )
                 rRoot.GetDoc().GetFilterEntriesArea(nCol, aRange.aStart.Row(),
                                                     aRange.aEnd.Row(), nTab, true, aFilterEntries);
 
+                // Excel has all filter values stored as forground colors
+                // Does not matter it is text color or cell background color
                 for (auto& rColor : aFilterEntries.getBackgroundColors())
                 {
-                    if (!maBackColorToDxfId.emplace(rColor, nColorIndex).second)
+                    if (!maColorToDxfId.emplace(rColor, nColorIndex).second)
                         continue;
 
-                    std::unique_ptr<XclExpCellArea> pExpCellArea(new XclExpCellArea(0, rColor));
+                    std::unique_ptr<XclExpCellArea> pExpCellArea(new XclExpCellArea(rColor, 0));
                     maDxf.push_back(std::make_unique<XclExpDxf>(rRoot, std::move(pExpCellArea)));
                     nColorIndex++;
                 }
                 for (auto& rColor : aFilterEntries.getTextColors())
                 {
-                    if (!maForeColorToDxfId.emplace(rColor, nColorIndex).second)
+                    if (!maColorToDxfId.emplace(rColor, nColorIndex).second)
                         continue;
 
                     std::unique_ptr<XclExpCellArea> pExpCellArea(new XclExpCellArea(rColor, 0));
@@ -3166,18 +3168,10 @@ sal_Int32 XclExpDxfs::GetDxfId( const OUString& rStyleName )
     return -1;
 }
 
-sal_Int32 XclExpDxfs::GetDxfByBackColor(Color& aColor)
-{
-    std::map<Color, sal_Int32>::iterator itr = maBackColorToDxfId.find(aColor);
-    if (itr != maBackColorToDxfId.end())
-        return itr->second;
-    return -1;
-}
-
-sal_Int32 XclExpDxfs::GetDxfByForeColor(Color& aColor)
+sal_Int32 XclExpDxfs::GetDxfByColor(Color& aColor)
 {
-    std::map<Color, sal_Int32>::iterator itr = maForeColorToDxfId.find(aColor);
-    if (itr != maForeColorToDxfId.end())
+    std::map<Color, sal_Int32>::iterator itr = maColorToDxfId.find(aColor);
+    if (itr != maColorToDxfId.end())
         return itr->second;
     return -1;
 }
diff --git a/sc/source/filter/inc/xestyle.hxx b/sc/source/filter/inc/xestyle.hxx
index 96e21b95a171..d111aee33c16 100644
--- a/sc/source/filter/inc/xestyle.hxx
+++ b/sc/source/filter/inc/xestyle.hxx
@@ -751,15 +751,13 @@ public:
     XclExpDxfs( const XclExpRoot& rRoot );
 
     sal_Int32 GetDxfId(const OUString& rName);
-    sal_Int32 GetDxfByBackColor(Color& aColor);
-    sal_Int32 GetDxfByForeColor(Color& aColor);
+    sal_Int32 GetDxfByColor(Color& aColor);
 
     virtual void SaveXml( XclExpXmlStream& rStrm) override;
 private:
     typedef std::vector< std::unique_ptr<XclExpDxf> > DxfContainer;
     std::map<OUString, sal_Int32> maStyleNameToDxfId;
-    std::map<Color, sal_Int32> maBackColorToDxfId;
-    std::map<Color, sal_Int32> maForeColorToDxfId;
+    std::map<Color, sal_Int32> maColorToDxfId;
     DxfContainer maDxf;
     std::unique_ptr<NfKeywordTable>   mpKeywordTable; /// Replacement table.
 };
diff --git a/sc/source/filter/oox/autofilterbuffer.cxx b/sc/source/filter/oox/autofilterbuffer.cxx
index f433456fb369..e2efd9a48a80 100644
--- a/sc/source/filter/oox/autofilterbuffer.cxx
+++ b/sc/source/filter/oox/autofilterbuffer.cxx
@@ -377,17 +377,9 @@ ApiFilterSettings ColorFilter::finalizeImport(sal_Int32 /*nMaxCount*/)
         return aSettings;
 
     const SfxItemSet& rItemSet = pStyleSheet->GetItemSet();
-    ::Color aColor;
-    if (mbIsBackgroundColor)
-    {
-        const SvxBrushItem* pItem = rItemSet.GetItem<SvxBrushItem>(ATTR_BACKGROUND);
-        aColor = pItem->GetColor();
-    }
-    else
-    {
-        const SvxColorItem* pItem = rItemSet.GetItem<SvxColorItem>(ATTR_FONT_COLOR);
-        aColor = pItem->GetValue();
-    }
+    // Color (whether text or background color) is always stored in ATTR_BACKGROUND
+    const SvxBrushItem* pItem = rItemSet.GetItem<SvxBrushItem>(ATTR_BACKGROUND);
+    ::Color aColor = pItem->GetColor();
     util::Color nColor(aColor);
     aSettings.appendField(true, nColor, mbIsBackgroundColor);
     return aSettings;
diff --git a/sc/source/filter/oox/stylesbuffer.cxx b/sc/source/filter/oox/stylesbuffer.cxx
index a6887b7bd103..db86ac8d0659 100644
--- a/sc/source/filter/oox/stylesbuffer.cxx
+++ b/sc/source/filter/oox/stylesbuffer.cxx
@@ -1824,11 +1824,14 @@ void Fill::finalizeImport()
         {
             if( rModel.mbFillColorUsed && (!rModel.mbPatternUsed || (rModel.mnPattern == XML_solid)) )
             {
-                rModel.maPatternColor = rModel.maFillColor;
+                if (!rModel.mbPatternUsed)
+                    rModel.maPatternColor = rModel.maFillColor;
                 rModel.mnPattern = XML_solid;
                 rModel.mbPattColorUsed = rModel.mbPatternUsed = true;
             }
-            else if( !rModel.mbFillColorUsed && rModel.mbPatternUsed && (rModel.mnPattern == XML_solid) )
+            else if(
+                !rModel.mbFillColorUsed && !rModel.mbPattColorUsed &&
+                rModel.mbPatternUsed && rModel.mnPattern == XML_solid )
             {
                 rModel.mbPatternUsed = false;
             }


More information about the Libreoffice-commits mailing list