sc/qa/unit/data/xlsx/autofilter-colors-fg.xlsx |binary
 sc/qa/unit/subsequent_export_test2.cxx         |   64 ++++++++++++++++---------
 sc/source/filter/excel/excrecds.cxx            |   15 +++--
 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, 70 insertions(+), 58 deletions(-)

New commits:
commit 6f908b48373b71d45c8119b296b0504fb586f6f8
Author:     Vasily Melenchuk <vasily.melenc...@cib.de>
AuthorDate: Fri Sep 24 15:18:13 2021 +0200
Commit:     Thorsten Behrens <thorsten.behr...@allotropia.de>
CommitDate: Fri Oct 1 14:00:46 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.mehrbr...@allotropia.de>
    
    Change-Id: Iacd352ae46bf84859dc15ee695b6dc63240afe7d
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122593
    Tested-by: Jenkins
    Reviewed-by: Thorsten Behrens <thorsten.behr...@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_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;
             }

Reply via email to