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

Reply via email to