sc/qa/unit/data/xlsx/shared-formula/refupdate.xlsx |binary sc/qa/unit/filters-test.cxx | 31 +++++++++++++++++++++ sc/source/filter/ftools/sharedformulagroups.cxx | 16 +++++++++- sc/source/filter/inc/sharedformulagroups.hxx | 24 +++++++++++++++- sc/source/filter/oox/formulabuffer.cxx | 23 ++++++++++++--- 5 files changed, 87 insertions(+), 7 deletions(-)
New commits: commit be02eeffd9bf49891c0fff0d358c1d8a2bd877bd Author: Dennis Francis <dennis.fran...@collabora.com> AuthorDate: Thu Nov 21 14:17:22 2019 +0530 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Wed Nov 27 14:52:19 2019 +0100 tdf#128894: unit test for the bugfix Reviewed-on: https://gerrit.libreoffice.org/83362 Tested-by: Jenkins Reviewed-by: Dennis Francis <dennis.fran...@collabora.com> (cherry picked from commit 626d1527267ab856e516f2424173104f781b8f09) Change-Id: Ic6d3910f12409f5af541c887760caefcb78b30f9 Reviewed-on: https://gerrit.libreoffice.org/83855 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Andras Timar <andras.ti...@collabora.com> diff --git a/sc/qa/unit/data/xlsx/shared-formula/refupdate.xlsx b/sc/qa/unit/data/xlsx/shared-formula/refupdate.xlsx new file mode 100644 index 000000000000..fa386d3a4384 Binary files /dev/null and b/sc/qa/unit/data/xlsx/shared-formula/refupdate.xlsx differ diff --git a/sc/qa/unit/filters-test.cxx b/sc/qa/unit/filters-test.cxx index 021eeb9e9952..a0738e9cf3b1 100644 --- a/sc/qa/unit/filters-test.cxx +++ b/sc/qa/unit/filters-test.cxx @@ -80,6 +80,7 @@ public: void testContentGnumeric(); void testSharedFormulaXLS(); void testSharedFormulaXLSX(); + void testSharedFormulaRefUpdateXLSX(); void testSheetNamesXLSX(); void testLegacyCellAnchoredRotatedShape(); void testEnhancedProtectionXLS(); @@ -104,6 +105,7 @@ public: CPPUNIT_TEST(testContentGnumeric); CPPUNIT_TEST(testSharedFormulaXLS); CPPUNIT_TEST(testSharedFormulaXLSX); + CPPUNIT_TEST(testSharedFormulaRefUpdateXLSX); CPPUNIT_TEST(testSheetNamesXLSX); CPPUNIT_TEST(testLegacyCellAnchoredRotatedShape); CPPUNIT_TEST(testEnhancedProtectionXLS); @@ -431,6 +433,35 @@ void ScFiltersTest::testSharedFormulaXLSX() xDocSh->DoClose(); } +void ScFiltersTest::testSharedFormulaRefUpdateXLSX() +{ + ScDocShellRef xDocSh = loadDoc("shared-formula/refupdate.", FORMAT_XLSX); + ScDocument& rDoc = xDocSh->GetDocument(); + sc::AutoCalcSwitch aACSwitch(rDoc, true); // turn auto calc on. + rDoc.DeleteRow(ScRange(0, 4, 0, MAXCOL, 4, 0)); // delete row 5. + + struct TestCase { + ScAddress aPos; + const char* pExpectedFormula; + const char* pErrorMsg; + }; + + TestCase aCases[4] = { + { ScAddress(1, 0, 0), "B29+1", "Wrong formula in B1" }, + { ScAddress(2, 0, 0), "C29+1", "Wrong formula in C1" }, + { ScAddress(3, 0, 0), "D29+1", "Wrong formula in D1" }, + { ScAddress(4, 0, 0), "E29+1", "Wrong formula in E1" }, + }; + + for (size_t nIdx = 0; nIdx < 4; ++nIdx) + { + TestCase& rCase = aCases[nIdx]; + ASSERT_FORMULA_EQUAL(rDoc, rCase.aPos, rCase.pExpectedFormula, rCase.pErrorMsg); + } + + xDocSh->DoClose(); +} + void ScFiltersTest::testSheetNamesXLSX() { ScDocShellRef xDocSh = loadDoc("sheet-names.", FORMAT_XLSX); commit 686462e52d0314fe9e14edd048fbe6f0894f620f Author: Dennis Francis <dennis.fran...@collabora.com> AuthorDate: Wed Nov 20 13:24:48 2019 +0530 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Wed Nov 27 14:51:54 2019 +0100 tdf#128894: xlsx-import : Do not share tokens between cells... which are part of a xlsx-shared-formula along a *row*. If we do, any reference-updates on these cells while editing will mess things up. For example a shared formula "=A30+1" used for a few cells in the first row (say, A1, B1, C1 and D1) and on deleting a row, say row#5, the reference update operation will decrement the row index of all tokens in A1, B1, C1 and D1. But if they share tokens, they all end up pointing to row#26 instead of row#29 as each cell is updated which amounts to decrementing 4 times instead of once. However shared formulas along columns are not affected by this bug, when tokens are shared since we use formula-groups which only keeps one copy of token array for the entire group and reference-update code is designed to correctly work with formula-groups. Reviewed-on: https://gerrit.libreoffice.org/83361 Tested-by: Jenkins Reviewed-by: Luboš Luňák <l.lu...@collabora.com> Reviewed-by: Eike Rathke <er...@redhat.com> (cherry picked from commit b30251ca0d102ced36799ee18d4bbcd9e8530fa0) Change-Id: Ic0fe84d12fef18fbf21658664e2b2b86409bca27 Reviewed-on: https://gerrit.libreoffice.org/83854 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Andras Timar <andras.ti...@collabora.com> diff --git a/sc/source/filter/ftools/sharedformulagroups.cxx b/sc/source/filter/ftools/sharedformulagroups.cxx index 73d74e9d66a3..b5736d14fb72 100644 --- a/sc/source/filter/ftools/sharedformulagroups.cxx +++ b/sc/source/filter/ftools/sharedformulagroups.cxx @@ -15,13 +15,25 @@ namespace sc { void SharedFormulaGroups::set( size_t nSharedId, ScTokenArray* pArray ) { - m_Store.insert(std::make_pair(nSharedId, std::unique_ptr<ScTokenArray>(pArray))); + m_Store.emplace(nSharedId, + SharedFormulaGroupEntry(pArray, ScAddress(ScAddress::INITIALIZE_INVALID))); +} + +void SharedFormulaGroups::set( size_t nSharedId, ScTokenArray* pArray, const ScAddress& rOrigin ) +{ + m_Store.emplace(nSharedId, SharedFormulaGroupEntry(pArray, rOrigin)); } const ScTokenArray* SharedFormulaGroups::get( size_t nSharedId ) const { StoreType::const_iterator const it = m_Store.find(nSharedId); - return it == m_Store.end() ? nullptr : it->second.get(); + return it == m_Store.end() ? nullptr : it->second.getTokenArray(); +} + +const SharedFormulaGroupEntry* SharedFormulaGroups::getEntry( size_t nSharedId ) const +{ + StoreType::const_iterator const it = m_Store.find(nSharedId); + return it == m_Store.end() ? nullptr : &(it->second); } } diff --git a/sc/source/filter/inc/sharedformulagroups.hxx b/sc/source/filter/inc/sharedformulagroups.hxx index 547a9610f471..f5966dc53a21 100644 --- a/sc/source/filter/inc/sharedformulagroups.hxx +++ b/sc/source/filter/inc/sharedformulagroups.hxx @@ -10,21 +10,43 @@ #ifndef INCLUDED_SC_SOURCE_FILTER_INC_SHAREDFORMULAGROUPS_HXX #define INCLUDED_SC_SOURCE_FILTER_INC_SHAREDFORMULAGROUPS_HXX +#include <address.hxx> + #include <memory> #include <map> + class ScTokenArray; namespace sc { +class SharedFormulaGroupEntry +{ +private: + std::unique_ptr<ScTokenArray> mpArray; + ScAddress maOrigin; + +public: + SharedFormulaGroupEntry(ScTokenArray* pArray, const ScAddress& rOrigin) + : mpArray(pArray) + , maOrigin(rOrigin) + { + } + + const ScTokenArray* getTokenArray() const { return mpArray.get(); } + const ScAddress& getOrigin() const { return maOrigin; } +}; + class SharedFormulaGroups { private: - typedef std::map<size_t, std::unique_ptr<ScTokenArray>> StoreType; + typedef std::map<size_t, SharedFormulaGroupEntry> StoreType; StoreType m_Store; public: void set( size_t nSharedId, ScTokenArray* pArray ); + void set( size_t nSharedId, ScTokenArray* pArray, const ScAddress& rOrigin ); const ScTokenArray* get( size_t nSharedId ) const; + const SharedFormulaGroupEntry* getEntry( size_t nSharedId ) const; }; } diff --git a/sc/source/filter/oox/formulabuffer.cxx b/sc/source/filter/oox/formulabuffer.cxx index 91bddc80ec76..cfa12d48b3ed 100644 --- a/sc/source/filter/oox/formulabuffer.cxx +++ b/sc/source/filter/oox/formulabuffer.cxx @@ -124,7 +124,7 @@ void applySharedFormulas( if (pArray) { aComp.CompileTokenArray(); // Generate RPN tokens. - aGroups.set(nId, pArray); + aGroups.set(nId, pArray, aPos); } } } @@ -135,11 +135,26 @@ void applySharedFormulas( for (const FormulaBuffer::SharedFormulaDesc& rDesc : rCells) { const ScAddress& aPos = rDesc.maAddress; - const ScTokenArray* pArray = aGroups.get(rDesc.mnSharedId); - if (!pArray) + const sc::SharedFormulaGroupEntry* pEntry = aGroups.getEntry(rDesc.mnSharedId); + if (!pEntry) continue; - ScFormulaCell* pCell = new ScFormulaCell(&rDoc.getDoc(), aPos, *pArray); + const ScTokenArray* pArray = pEntry->getTokenArray(); + assert(pArray); + const ScAddress& rOrigin = pEntry->getOrigin(); + assert(rOrigin.IsValid()); + + ScFormulaCell* pCell; + // In case of shared-formula along a row, do not let + // these cells share the same token objects. + // If we do, any reference-updates on these cells + // (while editing) will mess things up. Pass the cloned array as a + // pointer and not as reference to avoid any further allocation. + if (rOrigin.Col() != aPos.Col()) + pCell = new ScFormulaCell(&rDoc.getDoc(), aPos, pArray->Clone()); + else + pCell = new ScFormulaCell(&rDoc.getDoc(), aPos, *pArray); + rDoc.setFormulaCell(aPos, pCell); if (rDesc.maCellValue.isEmpty()) { _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits