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 <[email protected]> AuthorDate: Fri Sep 24 15:18:13 2021 +0200 Commit: Thorsten Behrens <[email protected]> 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 <[email protected]> Change-Id: Iacd352ae46bf84859dc15ee695b6dc63240afe7d Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122593 Tested-by: Jenkins Reviewed-by: Thorsten Behrens <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122969 Tested-by: Thorsten Behrens <[email protected]> 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; }
