[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-6.0' - 2 commits - sc/qa sc/source
Dennis Francis (via logerrit)
logerrit at kemper.freedesktop.org
Wed Nov 27 13:52:38 UTC 2019
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 | 22 ++++++++++++++
sc/source/filter/oox/formulabuffer.cxx | 23 ++++++++++++---
5 files changed, 85 insertions(+), 7 deletions(-)
New commits:
commit beccda79a92db402aeba577beadae8563953297c
Author: Dennis Francis <dennis.francis at collabora.com>
AuthorDate: Thu Nov 21 14:17:22 2019 +0530
Commit: Andras Timar <andras.timar at collabora.com>
CommitDate: Wed Nov 27 14:51:31 2019 +0100
tdf#128894: unit test for the bugfix
Reviewed-on: https://gerrit.libreoffice.org/83362
Tested-by: Jenkins
Reviewed-by: Dennis Francis <dennis.francis at collabora.com>
(cherry picked from commit 626d1527267ab856e516f2424173104f781b8f09)
Change-Id: Ic6d3910f12409f5af541c887760caefcb78b30f9
(cherry picked from commit b53374e26c1aa95461bbcdaf4a985ff4847b6e28)
Reviewed-on: https://gerrit.libreoffice.org/83882
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Andras Timar <andras.timar at 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 24250db69aed..5f86c6581746 100644
--- a/sc/qa/unit/filters-test.cxx
+++ b/sc/qa/unit/filters-test.cxx
@@ -77,6 +77,7 @@ public:
void testContentGnumeric();
void testSharedFormulaXLS();
void testSharedFormulaXLSX();
+ void testSharedFormulaRefUpdateXLSX();
void testSheetNamesXLSX();
void testLegacyCellAnchoredRotatedShape();
void testEnhancedProtectionXLS();
@@ -100,6 +101,7 @@ public:
CPPUNIT_TEST(testContentGnumeric);
CPPUNIT_TEST(testSharedFormulaXLS);
CPPUNIT_TEST(testSharedFormulaXLSX);
+ CPPUNIT_TEST(testSharedFormulaRefUpdateXLSX);
CPPUNIT_TEST(testSheetNamesXLSX);
CPPUNIT_TEST(testLegacyCellAnchoredRotatedShape);
CPPUNIT_TEST(testEnhancedProtectionXLS);
@@ -419,6 +421,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 541af6456cd2f0598378f772e25f621d3cf3f6bb
Author: Dennis Francis <dennis.francis at collabora.com>
AuthorDate: Wed Nov 20 13:24:48 2019 +0530
Commit: Andras Timar <andras.timar at collabora.com>
CommitDate: Wed Nov 27 14:51:18 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.lunak at collabora.com>
Reviewed-by: Eike Rathke <erack at redhat.com>
(cherry picked from commit b30251ca0d102ced36799ee18d4bbcd9e8530fa0)
(cherry picked from commit 5a1cb68d574e0788a0bad4859acdd5005babb860)
Change-Id: Ic0fe84d12fef18fbf21658664e2b2b86409bca27
Reviewed-on: https://gerrit.libreoffice.org/83881
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Andras Timar <andras.timar at collabora.com>
diff --git a/sc/source/filter/ftools/sharedformulagroups.cxx b/sc/source/filter/ftools/sharedformulagroups.cxx
index 1da53771d16f..47ac4475fbd1 100644
--- a/sc/source/filter/ftools/sharedformulagroups.cxx
+++ b/sc/source/filter/ftools/sharedformulagroups.cxx
@@ -14,13 +14,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 f59cae1b52ff..5472862a7c90 100644
--- a/sc/source/filter/inc/sharedformulagroups.hxx
+++ b/sc/source/filter/inc/sharedformulagroups.hxx
@@ -10,6 +10,7 @@
#ifndef INCLUDED_SC_SOURCE_FILTER_INC_SHAREDFORMULAGROUPS_HXX
#define INCLUDED_SC_SOURCE_FILTER_INC_SHAREDFORMULAGROUPS_HXX
+#include <address.hxx>
#include <tokenarray.hxx>
#include <memory>
@@ -17,15 +18,34 @@
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 48758f16e17b..286fd7a2d94f 100644
--- a/sc/source/filter/oox/formulabuffer.cxx
+++ b/sc/source/filter/oox/formulabuffer.cxx
@@ -127,7 +127,7 @@ void applySharedFormulas(
if (pArray)
{
aComp.CompileTokenArray(); // Generate RPN tokens.
- aGroups.set(nId, pArray);
+ aGroups.set(nId, pArray, aPos);
}
}
}
@@ -138,11 +138,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())
{
More information about the Libreoffice-commits
mailing list