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 <nemeth@numbertext.org>
Reviewed-by: László Németh <nemeth@numbertext.org>
(cherry picked from commit a5513cb45d90e0a1bfa0dfe39c0f090f1cda45de)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111923
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
(cherry picked from commit 46f5c61e937cc34d8d06991137c713f43c241735)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/114504
Tested-by: Gabor Kelemen <kelemen.gabor2@nisz.hu>
Reviewed-by: Gabor Kelemen <kelemen.gabor2@nisz.hu>

5 files changed, 75 insertions, 14 deletions

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
--- /dev/null
+++ b/sc/qa/unit/data/xlsx/tdf139928.xlsx
diff --git a/sc/qa/unit/subsequent_export-test.cxx b/sc/qa/unit/subsequent_export-test.cxx
index c13695c111d1..293cd295a36e 100644
--- a/sc/qa/unit/subsequent_export-test.cxx
+++ b/sc/qa/unit/subsequent_export-test.cxx
@@ -761,7 +761,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);
@@ -771,7 +771,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 ce093603ddb6..f2d17b8d5345 100644
--- a/sc/qa/unit/subsequent_filters-test.cxx
+++ b/sc/qa/unit/subsequent_filters-test.cxx
@@ -106,6 +106,7 @@ public:
     virtual void tearDown() override;
 
     //ods, xls, xlsx filter tests
+    void testCondFormatOperatorsSameRangeXLSX();
     void testCondFormatFormulaIsXLSX();
     void testCondFormatBeginsAndEndsWithXLSX();
     void testExtCondFormatXLSX();
@@ -278,6 +279,7 @@ public:
     void testDeleteCirclesInRowAndCol();
 
     CPPUNIT_TEST_SUITE(ScFiltersTest);
+    CPPUNIT_TEST(testCondFormatOperatorsSameRangeXLSX);
     CPPUNIT_TEST(testCondFormatFormulaIsXLSX);
     CPPUNIT_TEST(testCondFormatBeginsAndEndsWithXLSX);
     CPPUNIT_TEST(testExtCondFormatXLSX);
@@ -494,6 +496,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);
@@ -2492,7 +2528,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);
@@ -2502,7 +2538,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 8e4f3e5278b8..3ba90978e91f 100644
--- a/sc/source/filter/inc/extlstcontext.hxx
+++ b/sc/source/filter/inc/extlstcontext.hxx
@@ -41,6 +41,14 @@ private:
     bool mbFirstEntry;
 };
 
+struct ExtCondFormatRuleModel
+{
+    sal_Int32 nPriority;
+    ScConditionMode eOperator;
+    OUString aFormula;
+    OUString aStyle;
+};
+
 class ExtConditionalFormattingContext : public WorksheetContextBase
 {
 public:
@@ -52,16 +60,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 f06d2e55241f..95cc43fad441 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,30 @@ 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, "", pDoc,
-                                                                      rPos, rStyle, "", "",
+
+                    ScCondFormatEntry* pEntry = new ScCondFormatEntry(maModels[i].eOperator, maModels[i].aFormula, "", pDoc,
+                                                                      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();