diff options
author | Dennis Francis <dennis.francis@collabora.com> | 2019-04-19 23:15:53 +0530 |
---|---|---|
committer | Andras Timar <andras.timar@collabora.com> | 2019-05-04 10:22:02 +0200 |
commit | c2f1c68ffb6dfa1ce7de09dcc428d6c53549e88d (patch) | |
tree | 15d5df7332e6249922645e58d30c2706df17c659 /sc | |
parent | 8c637b47d9de4b3a64c33a9c2ffe7ed220be2467 (diff) |
tdf#122590: follow-up : import x14:cfRule priorities
If there are x:cfRule's and x14:cfRule's with matching
range-list, then insert the conditional-fmt entries into
the document in the order of the priorities. That is
don't just append the x14:cfRule entries to the document
after the x:cfRule entries are inserted.
There was also a off-by-one bug in the priority export
of x14:cfRule entries. This caused the priority numbers
to be duplicated. This is also fixed.
Change-Id: I5b0d11c4456b2966b808f6ee589075a870f43768
Reviewed-on: https://gerrit.libreoffice.org/71311
Tested-by: Jenkins
Reviewed-by: Andras Timar <andras.timar@collabora.com>
Diffstat (limited to 'sc')
-rw-r--r-- | sc/qa/unit/data/xlsx/conditional_fmt_checkpriority.xlsx | bin | 0 -> 10682 bytes | |||
-rw-r--r-- | sc/qa/unit/subsequent_export-test.cxx | 48 | ||||
-rw-r--r-- | sc/source/filter/excel/xeextlst.cxx | 2 | ||||
-rw-r--r-- | sc/source/filter/inc/condformatbuffer.hxx | 13 | ||||
-rw-r--r-- | sc/source/filter/inc/extlstcontext.hxx | 2 | ||||
-rw-r--r-- | sc/source/filter/oox/condformatbuffer.cxx | 87 | ||||
-rw-r--r-- | sc/source/filter/oox/extlstcontext.cxx | 15 |
7 files changed, 159 insertions, 8 deletions
diff --git a/sc/qa/unit/data/xlsx/conditional_fmt_checkpriority.xlsx b/sc/qa/unit/data/xlsx/conditional_fmt_checkpriority.xlsx Binary files differnew file mode 100644 index 000000000000..e9af11d00615 --- /dev/null +++ b/sc/qa/unit/data/xlsx/conditional_fmt_checkpriority.xlsx diff --git a/sc/qa/unit/subsequent_export-test.cxx b/sc/qa/unit/subsequent_export-test.cxx index f4fa59c0c827..de8eb723b884 100644 --- a/sc/qa/unit/subsequent_export-test.cxx +++ b/sc/qa/unit/subsequent_export-test.cxx @@ -104,6 +104,7 @@ public: void testDataBarExportXLSX(); void testConditionalFormatRangeListXLSX(); void testConditionalFormatContainsTextXLSX(); + void testConditionalFormatPriorityCheckXLSX(); void testMiscRowHeightExport(); void testNamedRangeBugfdo62729(); void testBuiltinRangesXLSX(); @@ -241,6 +242,7 @@ public: CPPUNIT_TEST(testDataBarExportXLSX); CPPUNIT_TEST(testConditionalFormatRangeListXLSX); CPPUNIT_TEST(testConditionalFormatContainsTextXLSX); + CPPUNIT_TEST(testConditionalFormatPriorityCheckXLSX); CPPUNIT_TEST(testMiscRowHeightExport); CPPUNIT_TEST(testNamedRangeBugfdo62729); CPPUNIT_TEST(testBuiltinRangesXLSX); @@ -380,6 +382,8 @@ void ScExportTest::registerNamespaces(xmlXPathContextPtr& pXmlXPathCtx) { BAD_CAST("number"), BAD_CAST("urn:oasis:names:tc:opendocument:xmlns:datastyle:1.0") }, { BAD_CAST("loext"), BAD_CAST("urn:org:documentfoundation:names:experimental:office:xmlns:loext:1.0") }, { BAD_CAST("ContentType"), BAD_CAST("http://schemas.openxmlformats.org/package/2006/content-types") }, + { BAD_CAST("x14"), BAD_CAST("http://schemas.microsoft.com/office/spreadsheetml/2009/9/main") }, + { BAD_CAST("xm"), BAD_CAST("http://schemas.microsoft.com/office/excel/2006/main") }, }; for(size_t i = 0; i < SAL_N_ELEMENTS(aNamespaces); ++i) { @@ -3928,6 +3932,50 @@ void ScExportTest::testConditionalFormatContainsTextXLSX() assertXPathContent(pDoc, "//x:conditionalFormatting/x:cfRule/x:formula", "NOT(ISERROR(SEARCH(\"test\",A1)))"); } +void ScExportTest::testConditionalFormatPriorityCheckXLSX() +{ + ScDocShellRef xDocSh = loadDoc("conditional_fmt_checkpriority.", FORMAT_XLSX); + CPPUNIT_ASSERT(xDocSh.is()); + + xmlDocPtr pDoc = XPathHelper::parseExport2(*this, *xDocSh, m_xSFactory, "xl/worksheets/sheet1.xml", FORMAT_XLSX); + CPPUNIT_ASSERT(pDoc); + + constexpr bool bHighPriorityExtensionA1 = true; // Should A1's extension cfRule has higher priority than normal cfRule ? + constexpr bool bHighPriorityExtensionA3 = false; // Should A3's extension cfRule has higher priority than normal cfRule ? + + size_t nA1NormalPriority = 0; + size_t nA1ExtPriority = 0; + size_t nA3NormalPriority = 0; + size_t nA3ExtPriority = 0; + + for (size_t nIdx = 1; nIdx <= 2; ++nIdx) + { + OString aIdx = OString::number(nIdx); + OUString aCellAddr = getXPath(pDoc, "//x:conditionalFormatting[" + aIdx + "]", "sqref"); + OUString aPriority = getXPath(pDoc, "//x:conditionalFormatting[" + aIdx + "]/x:cfRule", "priority");; + + CPPUNIT_ASSERT_MESSAGE("conditionalFormatting sqref must be either A1 or A3", aCellAddr == "A1" || aCellAddr == "A3"); + + if (aCellAddr == "A1") + nA1NormalPriority = aPriority.toUInt32(); + else + nA3NormalPriority = aPriority.toUInt32(); + + aCellAddr = getXPathContent(pDoc, "//x:extLst/x:ext[1]/x14:conditionalFormattings/x14:conditionalFormatting[" + aIdx + "]/xm:sqref"); + aPriority = getXPath(pDoc, "//x:extLst/x:ext[1]/x14:conditionalFormattings/x14:conditionalFormatting[" + aIdx + "]/x14:cfRule", "priority"); + + CPPUNIT_ASSERT_MESSAGE("x14:conditionalFormatting sqref must be either A1 or A3", aCellAddr == "A1" || aCellAddr == "A3"); + + if (aCellAddr == "A1") + nA1ExtPriority = aPriority.toUInt32(); + else + nA3ExtPriority = aPriority.toUInt32(); + } + + CPPUNIT_ASSERT_EQUAL_MESSAGE("Wrong priorities for A1", bHighPriorityExtensionA1, nA1ExtPriority < nA1NormalPriority); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Wrong priorities for A3", bHighPriorityExtensionA3, nA3ExtPriority < nA3NormalPriority); +} + void ScExportTest::testEscapeCharInNumberFormatXLSX() { ScDocShellRef xDocSh = loadDoc("tdf81939.", FORMAT_XLSX); diff --git a/sc/source/filter/excel/xeextlst.cxx b/sc/source/filter/excel/xeextlst.cxx index 69168fa8184e..2d74db8b25dd 100644 --- a/sc/source/filter/excel/xeextlst.cxx +++ b/sc/source/filter/excel/xeextlst.cxx @@ -397,7 +397,7 @@ void XclExpExtCfRule::SaveXml( XclExpXmlStream& rStrm ) sax_fastparser::FSHelperPtr& rWorksheet = rStrm.GetCurrentStream(); rWorksheet->startElementNS( XML_x14, XML_cfRule, XML_type, pType, - XML_priority, mnPriority == -1 ? nullptr : OString::number(mnPriority).getStr(), + XML_priority, mnPriority == -1 ? nullptr : OString::number(mnPriority + 1).getStr(), XML_operator, mOperator, XML_id, maId ); diff --git a/sc/source/filter/inc/condformatbuffer.hxx b/sc/source/filter/inc/condformatbuffer.hxx index d67f648e8086..8fbca5c0febf 100644 --- a/sc/source/filter/inc/condformatbuffer.hxx +++ b/sc/source/filter/inc/condformatbuffer.hxx @@ -160,6 +160,9 @@ public: /** Imports rule settings from a CFRULE record. */ void importCfRule( SequenceInputStream& rStrm ); + /** Directly set a ScFormatEntry with a priority ready for finalizeImport(). */ + void setFormatEntry(sal_Int32 nPriority, ScFormatEntry* pEntry); + /** Creates a conditional formatting rule in the Calc document. */ void finalizeImport(); @@ -174,6 +177,7 @@ private: const CondFormat& mrCondFormat; CondFormatRuleModel maModel; ScConditionalFormat* mpFormat; + ScFormatEntry* mpFormatEntry; std::unique_ptr<ColorScaleRule> mpColor; std::unique_ptr<DataBarRule> mpDataBar; std::unique_ptr<IconSetRule> mpIconSet; @@ -190,9 +194,12 @@ struct CondFormatModel explicit CondFormatModel(); }; +class CondFormatBuffer; + /** Represents a conditional formatting object with a list of affected cell ranges. */ class CondFormat : public WorksheetHelper { +friend class CondFormatBuffer; public: explicit CondFormat( const WorksheetHelper& rHelper ); @@ -268,14 +275,17 @@ public: class ExtCfCondFormat { public: - ExtCfCondFormat(const ScRangeList& aRange, std::vector< std::unique_ptr<ScFormatEntry> >& rEntries); + ExtCfCondFormat(const ScRangeList& aRange, std::vector< std::unique_ptr<ScFormatEntry> >& rEntries, + std::vector<sal_Int32>* pPriorities = nullptr); ~ExtCfCondFormat(); const ScRangeList& getRange(); const std::vector< std::unique_ptr<ScFormatEntry> >& getEntries(); + const std::vector<sal_Int32>& getPriorities() const { return maPriorities; } private: std::vector< std::unique_ptr<ScFormatEntry> > maEntries; + std::vector<sal_Int32> maPriorities; ScRangeList const maRange; }; @@ -307,6 +317,7 @@ private: CondFormatVec maCondFormats; /// All conditional formatting in a sheet. ExtCfDataBarRuleVec maCfRules; /// All external conditional formatting rules in a sheet. std::vector< std::unique_ptr<ExtCfCondFormat> > maExtCondFormats; + sal_Int32 mnNonPrioritizedRuleNextPriority = 1048576; }; } // namespace xls diff --git a/sc/source/filter/inc/extlstcontext.hxx b/sc/source/filter/inc/extlstcontext.hxx index a00e3236fad7..99cce913c2aa 100644 --- a/sc/source/filter/inc/extlstcontext.hxx +++ b/sc/source/filter/inc/extlstcontext.hxx @@ -54,11 +54,13 @@ public: private: 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; }; /** diff --git a/sc/source/filter/oox/condformatbuffer.cxx b/sc/source/filter/oox/condformatbuffer.cxx index 5750cce50592..0d75b4399200 100644 --- a/sc/source/filter/oox/condformatbuffer.cxx +++ b/sc/source/filter/oox/condformatbuffer.cxx @@ -18,6 +18,8 @@ */ #include <memory> +#include <unordered_set> +#include <unordered_map> #include <condformatbuffer.hxx> #include <formulaparser.hxx> @@ -424,7 +426,8 @@ void CondFormatRuleModel::setBiff12TextType( sal_Int32 nOperator ) CondFormatRule::CondFormatRule( const CondFormat& rCondFormat, ScConditionalFormat* pFormat ) : WorksheetHelper( rCondFormat ), mrCondFormat( rCondFormat ), - mpFormat(pFormat) + mpFormat(pFormat), + mpFormatEntry(nullptr) { } @@ -698,8 +701,20 @@ void CondFormatRule::importCfRule( SequenceInputStream& rStrm ) } } +void CondFormatRule::setFormatEntry(sal_Int32 nPriority, ScFormatEntry* pEntry) +{ + maModel.mnPriority = nPriority; + mpFormatEntry = pEntry; +} + void CondFormatRule::finalizeImport() { + if (mpFormatEntry) + { + mpFormat->AddEntry(mpFormatEntry); + return; + } + ScConditionMode eOperator = ScConditionMode::NONE; /* Replacement formula for unsupported rule types (text comparison rules, @@ -1091,10 +1106,63 @@ ScConditionalFormat* findFormatByRange(const ScRangeList& rRange, const ScDocume return nullptr; } +class ScRangeListHasher +{ +public: + size_t operator() (ScRangeList const& rRanges) const + { + size_t nHash = 0; + for (size_t nIdx = 0; nIdx < rRanges.size(); ++nIdx) + nHash += rRanges[nIdx].hashArea(); + return nHash; + } +}; + } void CondFormatBuffer::finalizeImport() { + std::unordered_set<size_t> aDoneExtCFs; + typedef std::unordered_map<ScRangeList, CondFormat*, ScRangeListHasher> RangeMap; + typedef std::vector<std::unique_ptr<ScFormatEntry>> FormatEntries; + RangeMap aRangeMap; + for (auto& rxCondFormat : maCondFormats) + { + if (aRangeMap.find(rxCondFormat->getRanges()) != aRangeMap.end()) + continue; + aRangeMap[rxCondFormat->getRanges()] = rxCondFormat.get(); + } + + size_t nExtCFIndex = 0; + for (const auto& rxExtCondFormat : maExtCondFormats) + { + ScDocument* pDoc = &getScDocument(); + const ScRangeList& rRange = rxExtCondFormat->getRange(); + RangeMap::iterator it = aRangeMap.find(rRange); + if (it != aRangeMap.end()) + { + CondFormat& rCondFormat = *it->second; + const FormatEntries& rEntries = rxExtCondFormat->getEntries(); + const std::vector<sal_Int32>& rPriorities = rxExtCondFormat->getPriorities(); + size_t nEntryIdx = 0; + for (const auto& rxEntry : rEntries) + { + CondFormatRuleRef xRule = rCondFormat.createRule(); + ScFormatEntry* pNewEntry = rxEntry->Clone(pDoc); + sal_Int32 nPriority = rPriorities[nEntryIdx]; + if (nPriority == -1) + nPriority = mnNonPrioritizedRuleNextPriority++; + xRule->setFormatEntry(nPriority, pNewEntry); + rCondFormat.insertRule(xRule); + ++nEntryIdx; + } + + aDoneExtCFs.insert(nExtCFIndex); + } + + ++nExtCFIndex; + } + for( const auto& rxCondFormat : maCondFormats ) { if ( rxCondFormat.get()) @@ -1106,10 +1174,16 @@ void CondFormatBuffer::finalizeImport() rxCfRule.get()->finalizeImport(); } + nExtCFIndex = 0; for (const auto& rxExtCondFormat : maExtCondFormats) { - ScDocument* pDoc = &getScDocument(); + if (aDoneExtCFs.count(nExtCFIndex)) + { + ++nExtCFIndex; + continue; + } + ScDocument* pDoc = &getScDocument(); const ScRangeList& rRange = rxExtCondFormat->getRange(); SCTAB nTab = rRange.front().aStart.Tab(); ScConditionalFormat* pFormat = findFormatByRange(rRange, pDoc, nTab); @@ -1128,6 +1202,8 @@ void CondFormatBuffer::finalizeImport() { pFormat->AddEntry(rxEntry->Clone(pDoc)); } + + ++nExtCFIndex; } rStyleIdx = 0; // Resets <extlst> <cfRule> style index. @@ -1294,10 +1370,15 @@ void ExtCfDataBarRule::importCfvo( const AttributeList& rAttribs ) maModel.maColorScaleType = rAttribs.getString( XML_type, OUString() ); } -ExtCfCondFormat::ExtCfCondFormat(const ScRangeList& rRange, std::vector< std::unique_ptr<ScFormatEntry> >& rEntries): +ExtCfCondFormat::ExtCfCondFormat(const ScRangeList& rRange, std::vector< std::unique_ptr<ScFormatEntry> >& rEntries, + std::vector<sal_Int32>* pPriorities): maRange(rRange) { maEntries.swap(rEntries); + if (pPriorities) + maPriorities = *pPriorities; + else + maPriorities.resize(maEntries.size(), -1); } ExtCfCondFormat::~ExtCfCondFormat() diff --git a/sc/source/filter/oox/extlstcontext.cxx b/sc/source/filter/oox/extlstcontext.cxx index 1d1bc8341fd5..46d268f32ca7 100644 --- a/sc/source/filter/oox/extlstcontext.cxx +++ b/sc/source/filter/oox/extlstcontext.cxx @@ -84,6 +84,7 @@ void ExtCfRuleContext::onStartElement( const AttributeList& rAttribs ) ExtConditionalFormattingContext::ExtConditionalFormattingContext(WorksheetContextBase& rFragment): WorksheetContextBase(rFragment) { + nPriority = -1; isPreviousElementF = false; } @@ -104,6 +105,7 @@ ContextHandlerRef ExtConditionalFormattingContext::onCreateContext(sal_Int32 nEl { OUString aType = rAttribs.getString(XML_type, OUString()); OUString aId = rAttribs.getString(XML_id, OUString()); + nPriority = rAttribs.getInteger( XML_priority, -1 ); if (aType == "dataBar") { @@ -179,6 +181,7 @@ void ExtConditionalFormattingContext::onEndElement() case XM_TOKEN(f): { rFormulas.push_back(aChars); + maPriorities.push_back(nPriority); } break; case XLS14_TOKEN( cfRule ): @@ -201,9 +204,9 @@ void ExtConditionalFormattingContext::onEndElement() aRange[i].aEnd.SetTab(nTab); } - if(isPreviousElementF) // sqref can be alone in some cases. + if (isPreviousElementF) // sqref can be alone in some cases. { - for(const OUString& rFormula : rFormulas) + for (const OUString& rFormula : rFormulas) { ScAddress rPos = aRange.GetTopLeftCorner(); rStyle = getStyles().createExtDxfStyle(rStyleIdx); @@ -215,11 +218,17 @@ void ExtConditionalFormattingContext::onEndElement() maEntries.push_back(std::unique_ptr<ScFormatEntry>(pEntry)); rStyleIdx++; } + + assert(rFormulas.size() == maPriorities.size()); rFormulas.clear(); } std::vector< std::unique_ptr<ExtCfCondFormat> >& rExtFormats = getCondFormats().importExtCondFormat(); - rExtFormats.push_back(std::make_unique<ExtCfCondFormat>(aRange, maEntries)); + rExtFormats.push_back(std::make_unique<ExtCfCondFormat>(aRange, maEntries, &maPriorities)); + + if (isPreviousElementF) + maPriorities.clear(); + isPreviousElementF = false; } break; |