sc/qa/unit/data/xlsx/tdf139928.xlsx |binary sc/qa/unit/subsequent_export-test.cxx | 4 +-- sc/qa/unit/subsequent_filters-test.cxx | 40 +++++++++++++++++++++++++++++++-- sc/source/filter/inc/extlstcontext.hxx | 12 ++++++++- sc/source/filter/oox/extlstcontext.cxx | 32 +++++++++++++++++++------- 5 files changed, 74 insertions(+), 14 deletions(-)
New commits: commit a5513cb45d90e0a1bfa0dfe39c0f090f1cda45de Author: Tibor Nagy <nagy.tib...@nisz.hu> AuthorDate: Sun Feb 7 21:24:45 2021 +0100 Commit: László Németh <nem...@numbertext.org> CommitDate: Mon Feb 15 13:06:51 2021 +0100 tdf#139928 XLSX import: fix conditional formatting in same cell range Multiple conditional formatting rules of the same cell range were imported incorrectly because of missing handling of their (different) priorities and operators. Note: older unit tests were modified according to the fixed priorities. Co-authored-by: Attila Szűcs (NISZ) Change-Id: I4b542b310642e1a85ef6281d0025b3ef2b2ba8c5 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/110544 Tested-by: László Németh <nem...@numbertext.org> Reviewed-by: László Németh <nem...@numbertext.org> diff --git a/sc/qa/unit/data/xlsx/tdf139928.xlsx b/sc/qa/unit/data/xlsx/tdf139928.xlsx new file mode 100644 index 000000000000..d0bc3067fa34 Binary files /dev/null and b/sc/qa/unit/data/xlsx/tdf139928.xlsx differ diff --git a/sc/qa/unit/subsequent_export-test.cxx b/sc/qa/unit/subsequent_export-test.cxx index 746eda5d3437..57d4cc45b53f 100644 --- a/sc/qa/unit/subsequent_export-test.cxx +++ b/sc/qa/unit/subsequent_export-test.cxx @@ -729,7 +729,7 @@ void ScExportTest::testCondFormatExportCellIs() CPPUNIT_ASSERT_EQUAL( ScConditionMode::Equal, pCondition->GetOperation()); OUString aStr = pCondition->GetExpression(ScAddress(0, 0, 0), 0); - CPPUNIT_ASSERT_EQUAL( OUString("$Sheet2.$A$1"), aStr ); + CPPUNIT_ASSERT_EQUAL( OUString("$Sheet2.$A$2"), aStr ); pEntry = pFormat->GetEntry(1); CPPUNIT_ASSERT(pEntry); @@ -739,7 +739,7 @@ void ScExportTest::testCondFormatExportCellIs() CPPUNIT_ASSERT_EQUAL( ScConditionMode::Equal, pCondition->GetOperation()); aStr = pCondition->GetExpression(ScAddress(0, 0, 0), 0); - CPPUNIT_ASSERT_EQUAL( OUString("$Sheet2.$A$2"), aStr ); + CPPUNIT_ASSERT_EQUAL( OUString("$Sheet2.$A$1"), aStr ); xDocSh->DoClose(); } diff --git a/sc/qa/unit/subsequent_filters-test.cxx b/sc/qa/unit/subsequent_filters-test.cxx index 7e7c62500674..b70777b1c176 100644 --- a/sc/qa/unit/subsequent_filters-test.cxx +++ b/sc/qa/unit/subsequent_filters-test.cxx @@ -108,6 +108,7 @@ public: virtual void tearDown() override; //ods, xls, xlsx filter tests + void testCondFormatOperatorsSameRangeXLSX(); void testCondFormatFormulaIsXLSX(); void testCondFormatBeginsAndEndsWithXLSX(); void testExtCondFormatXLSX(); @@ -307,6 +308,7 @@ public: void testTdf129940(); CPPUNIT_TEST_SUITE(ScFiltersTest); + CPPUNIT_TEST(testCondFormatOperatorsSameRangeXLSX); CPPUNIT_TEST(testCondFormatFormulaIsXLSX); CPPUNIT_TEST(testCondFormatBeginsAndEndsWithXLSX); CPPUNIT_TEST(testExtCondFormatXLSX); @@ -551,6 +553,40 @@ void testRangeNameImpl(const ScDocument& rDoc) } +void ScFiltersTest::testCondFormatOperatorsSameRangeXLSX() +{ + ScDocShellRef xDocSh = loadDoc(u"tdf139928.", FORMAT_XLSX); + CPPUNIT_ASSERT_MESSAGE("Failed to load tdf139928.xlsx", xDocSh.is()); + + ScDocument& rDoc = xDocSh->GetDocument(); + + ScConditionalFormat* pFormat = rDoc.GetCondFormat(0, 0, 0); + CPPUNIT_ASSERT(pFormat); + + const ScFormatEntry* pEntry = pFormat->GetEntry(0); + CPPUNIT_ASSERT(pEntry); + CPPUNIT_ASSERT_EQUAL(ScFormatEntry::Type::ExtCondition, pEntry->GetType()); + + const ScCondFormatEntry* pCondition = static_cast<const ScCondFormatEntry*>(pEntry); + CPPUNIT_ASSERT_EQUAL( ScConditionMode::ContainsText, pCondition->GetOperation()); + + pEntry = pFormat->GetEntry(1); + CPPUNIT_ASSERT(pEntry); + CPPUNIT_ASSERT_EQUAL(ScFormatEntry::Type::ExtCondition, pEntry->GetType()); + + pCondition = static_cast<const ScCondFormatEntry*>(pEntry); + CPPUNIT_ASSERT_EQUAL( ScConditionMode::BeginsWith, pCondition->GetOperation()); + + pEntry = pFormat->GetEntry(2); + CPPUNIT_ASSERT(pEntry); + CPPUNIT_ASSERT_EQUAL(ScFormatEntry::Type::ExtCondition, pEntry->GetType()); + + pCondition = static_cast<const ScCondFormatEntry*>(pEntry); + CPPUNIT_ASSERT_EQUAL( ScConditionMode::EndsWith, pCondition->GetOperation()); + + xDocSh->DoClose(); +} + void ScFiltersTest::testCondFormatFormulaIsXLSX() { ScDocShellRef xDocSh = loadDoc(u"tdf113013.", FORMAT_XLSX); @@ -2566,7 +2602,7 @@ void ScFiltersTest::testCondFormatImportCellIs() CPPUNIT_ASSERT_EQUAL( ScConditionMode::Equal, pCondition->GetOperation()); OUString aStr = pCondition->GetExpression(ScAddress(0, 0, 0), 0); - CPPUNIT_ASSERT_EQUAL( OUString("$Sheet2.$A$1"), aStr ); + CPPUNIT_ASSERT_EQUAL( OUString("$Sheet2.$A$2"), aStr ); pEntry = pFormat->GetEntry(1); CPPUNIT_ASSERT(pEntry); @@ -2576,7 +2612,7 @@ void ScFiltersTest::testCondFormatImportCellIs() CPPUNIT_ASSERT_EQUAL( ScConditionMode::Equal, pCondition->GetOperation()); aStr = pCondition->GetExpression(ScAddress(0, 0, 0), 0); - CPPUNIT_ASSERT_EQUAL( OUString("$Sheet2.$A$2"), aStr ); + CPPUNIT_ASSERT_EQUAL( OUString("$Sheet2.$A$1"), aStr ); xDocSh->DoClose(); } diff --git a/sc/source/filter/inc/extlstcontext.hxx b/sc/source/filter/inc/extlstcontext.hxx index 0f85d5e6f2d4..5106ffc98928 100644 --- a/sc/source/filter/inc/extlstcontext.hxx +++ b/sc/source/filter/inc/extlstcontext.hxx @@ -40,6 +40,14 @@ private: bool mbFirstEntry; }; +struct ExtCondFormatRuleModel +{ + sal_Int32 nPriority; + ScConditionMode eOperator; + OUString aFormula; + OUString aStyle; +}; + class ExtConditionalFormattingContext : public WorksheetContextBase { public: @@ -51,16 +59,16 @@ public: virtual void onEndElement() override; private: + ExtCondFormatRuleModel maModel; sal_Int32 nFormulaCount; OUString aChars; // Characters of between xml elements. - OUString rStyle; // Style of the corresponding condition sal_Int32 nPriority; // Priority of last cfRule element. ScConditionMode eOperator; // Used only when cfRule type is "cellIs" bool isPreviousElementF; // Used to distinguish alone <sqref> from <f> and <sqref> std::vector<std::unique_ptr<ScFormatEntry> > maEntries; - std::vector< OUString > rFormulas; // It holds formulas for a range, there can be more formula for same range. std::unique_ptr<IconSetRule> mpCurrentRule; std::vector<sal_Int32> maPriorities; + std::vector<ExtCondFormatRuleModel> maModels; }; /** diff --git a/sc/source/filter/oox/extlstcontext.cxx b/sc/source/filter/oox/extlstcontext.cxx index e0c41c25dc8d..eee453a7242c 100644 --- a/sc/source/filter/oox/extlstcontext.cxx +++ b/sc/source/filter/oox/extlstcontext.cxx @@ -125,6 +125,7 @@ ContextHandlerRef ExtConditionalFormattingContext::onCreateContext(sal_Int32 nEl OUString aId = rAttribs.getString(XML_id, OUString()); nPriority = rAttribs.getInteger( XML_priority, -1 ); maPriorities.push_back(nPriority); + maModel.nPriority = nPriority; if (aType == "dataBar") { @@ -151,31 +152,37 @@ ContextHandlerRef ExtConditionalFormattingContext::onCreateContext(sal_Int32 nEl { sal_Int32 aToken = rAttribs.getToken( XML_operator, XML_TOKEN_INVALID ); eOperator = CondFormatBuffer::convertToInternalOperator(aToken); + maModel.eOperator = eOperator; return this; } else if (aType == "containsText") { eOperator = ScConditionMode::ContainsText; + maModel.eOperator = eOperator; return this; } else if (aType == "notContainsText") { eOperator = ScConditionMode::NotContainsText; + maModel.eOperator = eOperator; return this; } else if (aType == "beginsWith") { eOperator = ScConditionMode::BeginsWith; + maModel.eOperator = eOperator; return this; } else if (aType == "endsWith") { eOperator = ScConditionMode::EndsWith; + maModel.eOperator = eOperator; return this; } else if (aType == "expression") { eOperator = ScConditionMode::Direct; + maModel.eOperator = eOperator; return this; } else @@ -227,13 +234,16 @@ void ExtConditionalFormattingContext::onEndElement() case XM_TOKEN(f): { if(!IsSpecificTextCondMode(eOperator) || nFormulaCount == 2) - rFormulas.push_back(aChars); + maModel.aFormula = aChars; } break; case XLS14_TOKEN( cfRule ): { getStyles().getExtDxfs().forEachMem( &Dxf::finalizeImport ); + maModel.aStyle = getStyles().createExtDxfStyle(rStyleIdx); + rStyleIdx++; nFormulaCount = 0; + maModels.push_back(maModel); } break; case XM_TOKEN(sqref): @@ -251,23 +261,29 @@ void ExtConditionalFormattingContext::onEndElement() aRange[i].aEnd.SetTab(nTab); } + if (maModels.size() > 1) + { + std::sort(maModels.begin(), maModels.end(), + [](const ExtCondFormatRuleModel& lhs, const ExtCondFormatRuleModel& rhs) { + return lhs.nPriority < rhs.nPriority; + }); + } + if (isPreviousElementF) // sqref can be alone in some cases. { - for (const OUString& rFormula : rFormulas) + for (size_t i = 0; i < maModels.size(); ++i) { ScAddress rPos = aRange.GetTopLeftCorner(); - rStyle = getStyles().createExtDxfStyle(rStyleIdx); - ScCondFormatEntry* pEntry = new ScCondFormatEntry(eOperator, rFormula, "", rDoc, - rPos, rStyle, "", "", + ScCondFormatEntry* pEntry = new ScCondFormatEntry(maModels[i].eOperator, maModels[i].aFormula, "", rDoc, + rPos, maModels[i].aStyle, "", "", formula::FormulaGrammar::GRAM_OOXML , formula::FormulaGrammar::GRAM_OOXML, ScFormatEntry::Type::ExtCondition ); maEntries.push_back(std::unique_ptr<ScFormatEntry>(pEntry)); - rStyleIdx++; } - assert(rFormulas.size() == maPriorities.size()); - rFormulas.clear(); + assert(maModels.size() == maPriorities.size()); + maModels.clear(); } std::vector< std::unique_ptr<ExtCfCondFormat> >& rExtFormats = getCondFormats().importExtCondFormat(); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits