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 eab60b2663596220cc4d33676524adebc8349a89
Author: Vasily Melenchuk <vasily.melenc...@cib.de>
AuthorDate: Fri Sep 24 15:18:13 2021 +0200
Commit: Samuel Mehrbrodt <samuel.mehrbr...@allotropia.de>
CommitDate: Mon Oct 4 09:48:21 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>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122916
Reviewed-by: Samuel Mehrbrodt <samuel.mehrbr...@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 dc72c46aebf4..0518cd11c154 100644
--- a/sc/qa/unit/subsequent_export-test2.cxx
+++ b/sc/qa/unit/subsequent_export-test2.cxx
@@ -119,7 +119,6 @@ public:
void testDateAutofilterODS();
void testAutofilterColorsODF();
void testAutofilterColorsOOXML();
- void testAutofilterColorsStyleOOXML();
void testAutofilterTop10XLSX();
void testRefStringXLSX();
@@ -222,7 +221,6 @@ public:
CPPUNIT_TEST(testDateAutofilterODS);
CPPUNIT_TEST(testAutofilterColorsODF);
CPPUNIT_TEST(testAutofilterColorsOOXML);
- CPPUNIT_TEST(testAutofilterColorsStyleOOXML);
CPPUNIT_TEST(testAutofilterTop10XLSX);
CPPUNIT_TEST(testRefStringXLSX);
@@ -763,27 +761,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 e0aec39a5173..a41621464051 100644
--- a/sc/source/filter/excel/excrecds.cxx
+++ b/sc/source/filter/excel/excrecds.cxx
@@ -849,13 +849,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 8abd140e2b9f..627cc93b7a6c 100644
--- a/sc/source/filter/excel/xestyle.cxx
+++ b/sc/source/filter/excel/xestyle.cxx
@@ -3069,18 +3069,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));
@@ -3181,18 +3183,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 70bd9b7fa0b2..52afbd9c5673 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;
}
