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@allotropia.de>

Change-Id: Iacd352ae46bf84859dc15ee695b6dc63240afe7d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122593
Tested-by: Jenkins
Reviewed-by: Thorsten Behrens <thorsten.behrens@allotropia.de>

7 files changed, 70 insertions, 58 deletions

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
--- /dev/null
+++ b/sc/qa/unit/data/xlsx/autofilter-colors-fg.xlsx
diff --git a/sc/qa/unit/subsequent_export_test2.cxx b/sc/qa/unit/subsequent_export_test2.cxx
index 10743521d31c..1ca736fe07ef 100644
--- a/sc/qa/unit/subsequent_export_test2.cxx
+++ b/sc/qa/unit/subsequent_export_test2.cxx
@@ -120,7 +120,6 @@ public:
     void testDateAutofilterODS();
     void testAutofilterColorsODF();
     void testAutofilterColorsOOXML();
-    void testAutofilterColorsStyleOOXML();
     void testAutofilterTop10XLSX();
 
     void testRefStringXLSX();
@@ -228,7 +227,6 @@ public:
     CPPUNIT_TEST(testDateAutofilterODS);
     CPPUNIT_TEST(testAutofilterColorsODF);
     CPPUNIT_TEST(testAutofilterColorsOOXML);
-    CPPUNIT_TEST(testAutofilterColorsStyleOOXML);
     CPPUNIT_TEST(testAutofilterTop10XLSX);
 
     CPPUNIT_TEST(testRefStringXLSX);
@@ -793,27 +791,49 @@ void ScExportTest2::testAutofilterColorsODF()
 
 void ScExportTest2::testAutofilterColorsOOXML()
 {
-    ScDocShellRef xDocSh = loadDoc(u"autofilter-colors.", FORMAT_XLSX);
-    CPPUNIT_ASSERT(xDocSh.is());
-
-    xmlDocUniquePtr 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 ScExportTest2::testAutofilterColorsStyleOOXML()
-{
-    ScDocShellRef xDocSh = loadDoc(u"autofilter-colors.", FORMAT_XLSX);
-    CPPUNIT_ASSERT(xDocSh.is());
-
-    xmlDocUniquePtr 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);
+        xmlDocUniquePtr 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;
+
+        xmlDocUniquePtr 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);
+        xmlDocUniquePtr 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;
+
+        xmlDocUniquePtr 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 ScExportTest2::testAutofilterTop10XLSX()
diff --git a/sc/source/filter/excel/excrecds.cxx b/sc/source/filter/excel/excrecds.cxx
index a09312d665de..65edd87ee5bc 100644
--- a/sc/source/filter/excel/excrecds.cxx
+++ b/sc/source/filter/excel/excrecds.cxx
@@ -850,13 +850,18 @@ void XclExpAutofilter::SaveXml( XclExpXmlStream& rStrm )
             if (!maColorValues.empty())
             {
                 Color color = maColorValues[0].first;
-                sal_Int32 nDxfId;
+                rtl::Reference<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)));
+                rWorksheet->singleElement(XML_colorFilter, pAttrList);
             }
         }
         break;
diff --git a/sc/source/filter/excel/xestyle.cxx b/sc/source/filter/excel/xestyle.cxx
index e9eccbfabc05..7032a2667a5b 100644
--- a/sc/source/filter/excel/xestyle.cxx
+++ b/sc/source/filter/excel/xestyle.cxx
@@ -3070,18 +3070,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));
@@ -3182,18 +3184,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 d7588c72067a..ec68176d3195 100644
--- a/sc/source/filter/inc/xestyle.hxx
+++ b/sc/source/filter/inc/xestyle.hxx
@@ -749,15 +749,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 ab0ff9a43c68..951347b62fcd 100644
--- a/sc/source/filter/oox/autofilterbuffer.cxx
+++ b/sc/source/filter/oox/autofilterbuffer.cxx
@@ -435,17 +435,9 @@ ApiFilterSettings ColorFilter::finalizeImport()
         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 b0c574f09378..7b02c2a36dcf 100644
--- a/sc/source/filter/oox/stylesbuffer.cxx
+++ b/sc/source/filter/oox/stylesbuffer.cxx
@@ -1825,11 +1825,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;
             }