[Libreoffice-commits] core.git: sc/inc sc/qa sc/source

scito (via logerrit) logerrit at kemper.freedesktop.org
Fri May 21 15:02:03 UTC 2021


 sc/inc/table.hxx                  |    9 +++++--
 sc/qa/unit/ucalc_copypaste.cxx    |   44 ++++++++++++++++++++++++++++++++++++++
 sc/source/core/data/clipparam.cxx |   10 ++++++--
 sc/source/core/data/document.cxx  |   11 ++++-----
 sc/source/core/data/table2.cxx    |   36 +++++++++++++++++--------------
 5 files changed, 82 insertions(+), 28 deletions(-)

New commits:
commit de4c23fb38a7848e1030075b9c15cbb5c558694c
Author:     scito <info at scito.ch>
AuthorDate: Fri May 21 13:11:51 2021 +0200
Commit:     Mike Kaganski <mike.kaganski at collabora.com>
CommitDate: Fri May 21 17:01:24 2021 +0200

    tdf#68976 cut paste transposed: fix wrong position in clipdoc
    
    The transposed cells were written always in A1 of the transposed clip.
    This is no problem for copy paste transposed. However, this is an issue for
    cut paste transposed as references of formulas pointing to cut cells are
    adjusted according to position changes.
    The solution is to write the transposed cells in the same top left
    corner of the transposed clipdoc as in the original non-transposed clipdoc.
    
    There are still issues with cut paste transposed: Formula references are
    not updated correctly in some cases (tdf#142065, tdf#142201).
    Comprehensive unit tests will follow with fixes for tdf#142065 and
    tdf#142201.
    
    Change-Id: Id691fabf89d7f25e05182b20bd379299fc18fbb8
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115535
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>

diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index 8d938e6e5ce1..cbf97d7dbd36 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -541,12 +541,15 @@ public:
     void        CopyConditionalFormat( SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2,
                             SCCOL nDx, SCROW nDy, const ScTable* pTable);
     /**
+     * @param nCombinedStartRow start row of the combined range;
+     * used for transposed multi range selection with row direction;
+     * for other cases than multi range row selection this it equal to nRow1
      * @param nRowDestOffset adjustment of destination row position;
      * used for transposed multi range selection with row direction, otherwise 0
      */
-    void TransposeClip(SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2, SCROW nRowDestOffset,
-                       ScTable* pTransClip, InsertDeleteFlags nFlags, bool bAsLink,
-                       bool bIncludeFiltered);
+    void TransposeClip(SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2, SCROW nCombinedStartRow,
+                       SCROW nRowDestOffset, ScTable* pTransClip, InsertDeleteFlags nFlags,
+                       bool bAsLink, bool bIncludeFiltered);
 
     // mark of this document
     void MixMarked(
diff --git a/sc/qa/unit/ucalc_copypaste.cxx b/sc/qa/unit/ucalc_copypaste.cxx
index 0bbd326eb128..35c1bb72f276 100644
--- a/sc/qa/unit/ucalc_copypaste.cxx
+++ b/sc/qa/unit/ucalc_copypaste.cxx
@@ -114,6 +114,7 @@ public:
     void testCopyPasteFormulas();
     void testCopyPasteFormulasExternalDoc();
     void testCopyPasteReferencesExternalDoc(); // tdf#106456
+    void testTdf68976();
     void testTdf71058();
 
     CPPUNIT_TEST_SUITE(TestCopyPaste);
@@ -189,6 +190,7 @@ public:
     CPPUNIT_TEST(testCopyPasteFormulasExternalDoc);
     CPPUNIT_TEST(testCopyPasteReferencesExternalDoc);
 
+    CPPUNIT_TEST(testTdf68976);
     CPPUNIT_TEST(testTdf71058);
 
     CPPUNIT_TEST_SUITE_END();
@@ -6820,6 +6822,48 @@ void TestCopyPaste::testCopyPasteReferencesExternalDoc()
     xExtDocSh->DoClose();
 }
 
+void TestCopyPaste::testTdf68976()
+{
+    const SCTAB nTab = 0;
+    m_pDoc->InsertTab(nTab, "Test");
+
+    m_pDoc->SetValue(0, 0, nTab, 1.0); // A1
+    m_pDoc->SetString(0, 1, nTab, "=$A$1"); // A2
+    m_pDoc->SetValue(0, 2, nTab, 1000.0); // A3
+
+    // Cut A3 to the clip document.
+    ScDocument aClipDoc(SCDOCMODE_CLIP);
+    ScRange aSrcRange(0, 2, nTab, 0, 2, nTab);
+    cutToClip(*m_xDocShell, aSrcRange, &aClipDoc, false); // A3
+
+    ScRange aDestRange(1, 3, nTab, 1, 3, nTab); // B4
+    ScMarkData aDestMark(m_pDoc->GetSheetLimits());
+
+    // Transpose
+    ScDocument* pOrigClipDoc = &aClipDoc;
+    ScDocumentUniquePtr pTransClip(new ScDocument(SCDOCMODE_CLIP));
+    aClipDoc.TransposeClip(pTransClip.get(), InsertDeleteFlags::ALL, false, true);
+    aDestMark.SetMarkArea(aDestRange);
+    // Paste
+    m_pDoc->CopyFromClip(aDestRange, aDestMark, InsertDeleteFlags::ALL, nullptr, pTransClip.get(),
+                         true, false, true, false);
+    m_pDoc->UpdateTranspose(aDestRange.aStart, pOrigClipDoc, aDestMark, nullptr);
+    pTransClip.reset();
+
+    // Check results
+    CPPUNIT_ASSERT_EQUAL(1.0, m_pDoc->GetValue(0, 0, nTab)); // A1
+    // Without the fix in place, this would have failed with
+    // - Expected: =$A$1
+    // - Actual  : =$B$4
+    ASSERT_FORMULA_EQUAL(*m_pDoc, ScAddress(0, 1, nTab), "$A$1", "Wrong formula");
+    // Without the fix in place, this would have failed with
+    // - Expected: 1
+    // - Actual  : 1000
+    CPPUNIT_ASSERT_EQUAL(1.0, m_pDoc->GetValue(0, 1, nTab)); // A2
+    CPPUNIT_ASSERT_EQUAL(0.0, m_pDoc->GetValue(0, 2, nTab)); // A3
+    CPPUNIT_ASSERT_EQUAL(1000.0, m_pDoc->GetValue(1, 3, nTab)); // B4
+}
+
 void TestCopyPaste::testTdf71058()
 {
     const SCTAB nTab = 0;
diff --git a/sc/source/core/data/clipparam.cxx b/sc/source/core/data/clipparam.cxx
index 48840605364c..1e8039deb4f5 100644
--- a/sc/source/core/data/clipparam.cxx
+++ b/sc/source/core/data/clipparam.cxx
@@ -153,7 +153,9 @@ void ScClipParam::transpose(const ScDocument& rSrcDoc, bool bIncludeFiltered,
                 nCol2 += static_cast<SCCOL>(nRowDelta);
                 nRow1 += static_cast<SCROW>(nColDelta);
                 nRow2 += static_cast<SCROW>(nColDelta);
-                aNewRanges.push_back( ScRange(nCol1, nRow1, rRange.aStart.Tab(), nCol2, nRow2, rRange.aStart.Tab() ) );
+                aNewRanges.push_back(ScRange(nColOrigin + nCol1, nRowOrigin + nRow1,
+                                             rRange.aStart.Tab(), nColOrigin + nCol2,
+                                             nRowOrigin + nRow2, rRange.aStart.Tab()));
             }
             else
                 nRowCount += nNonFilteredRows;
@@ -163,6 +165,7 @@ void ScClipParam::transpose(const ScDocument& rSrcDoc, bool bIncludeFiltered,
         // and selection are in the same dimension (i.e. row), see ScDocument::TransposeClip()
         if (bIsMultiRangeRowFilteredTranspose)
         {
+            assert(!bIncludeFiltered && "bIsMultiRangeRowFilteredTranspose can only be true if bIncludeFiltered is false");
             SCCOL nColDelta = rRange1.aStart.Col() - nColOrigin;
             SCROW nRowDelta = rRange1.aStart.Row() - nRowOrigin;
             SCCOL nCol1 = 0;
@@ -173,8 +176,9 @@ void ScClipParam::transpose(const ScDocument& rSrcDoc, bool bIncludeFiltered,
             nCol2 += static_cast<SCCOL>(nRowDelta);
             nRow1 += static_cast<SCROW>(nColDelta);
             nRow2 += static_cast<SCROW>(nColDelta);
-            aNewRanges.push_back(
-                ScRange(nCol1, nRow1, rRange1.aStart.Tab(), nCol2, nRow2, rRange1.aStart.Tab()));
+            aNewRanges.push_back(ScRange(nColOrigin + nCol1, nRowOrigin + nRow1,
+                                         rRange1.aStart.Tab(), nColOrigin + nCol2,
+                                         nRowOrigin + nRow2, rRange1.aStart.Tab()));
         }
     }
     maRanges = aNewRanges;
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index 012913d37924..cbce2c2a824b 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -2367,8 +2367,7 @@ void ScDocument::TransposeClip(ScDocument* pTransClip, InsertDeleteFlags nFlags,
             SCROW nRowCountNonFiltered = CountNonFilteredRows(
                 aClipRange.aStart.Row(), aClipRange.aEnd.Row(), aClipRange.aStart.Tab());
             assert(!bIncludeFiltered && "bIsMultiRangeRowFilteredTranspose can only be true if bIncludeFiltered is false");
-            SCROW nRowCountInRange = nRowCountNonFiltered;
-            nRowCount += nRowCountInRange; // for next iteration
+            nRowCount += nRowCountNonFiltered; // for next iteration
         }
 
         for (SCTAB i = 0; i < static_cast<SCTAB>(maTabs.size()); i++)
@@ -2376,10 +2375,10 @@ void ScDocument::TransposeClip(ScDocument* pTransClip, InsertDeleteFlags nFlags,
             if (maTabs[i])
             {
                 OSL_ENSURE(pTransClip->maTabs[i], "TransposeClip: Table not there");
-                maTabs[i]->TransposeClip(aClipRange.aStart.Col(), aClipRange.aStart.Row(),
-                                         aClipRange.aEnd.Col(), aClipRange.aEnd.Row(), nRowOffset,
-                                         pTransClip->maTabs[i].get(), nFlags, bAsLink,
-                                         bIncludeFiltered);
+                maTabs[i]->TransposeClip(
+                    aClipRange.aStart.Col(), aClipRange.aStart.Row(), aClipRange.aEnd.Col(),
+                    aClipRange.aEnd.Row(), aCombinedClipRange.aStart.Row(), nRowOffset,
+                    pTransClip->maTabs[i].get(), nFlags, bAsLink, bIncludeFiltered);
 
                 if ( mpDrawLayer && ( nFlags & InsertDeleteFlags::OBJECTS ) )
                 {
diff --git a/sc/source/core/data/table2.cxx b/sc/source/core/data/table2.cxx
index 47afc5886ab1..3ae7c49c9d5f 100644
--- a/sc/source/core/data/table2.cxx
+++ b/sc/source/core/data/table2.cxx
@@ -798,6 +798,7 @@ class TransClipHandler
     ScTable& mrClipTab;
     const ScTable& mrSrcTab;
     SCTAB mnSrcTab;
+    SCCOL mnCol1;
     SCCOL mnSrcCol;
     size_t mnTopRow;
     size_t mnEndRow;
@@ -811,7 +812,8 @@ class TransClipHandler
 
     ScAddress getDestPos(size_t nRow) const
     {
-        return ScAddress(static_cast<SCCOL>(nRow-mnTopRow), mnTransRow, mrClipTab.GetTab());
+        return ScAddress(static_cast<SCCOL>(mnCol1 + nRow - mnTopRow), mnTransRow,
+                         mrClipTab.GetTab());
     }
 
     ScFormulaCell* createRefCell(size_t nSrcRow, const ScAddress& rDestPos) const
@@ -828,23 +830,25 @@ class TransClipHandler
 
     void setLink(size_t nRow)
     {
-        SCCOL nTransCol = nRow - mnTopRow - mnFilteredRows + mnRowDestOffset;
+        SCCOL nTransCol = mnCol1 + nRow - mnTopRow - mnFilteredRows + mnRowDestOffset;
         mrClipTab.SetFormulaCell(nTransCol, mnTransRow,
                                  createRefCell(nRow, getDestPos(nRow)));
     }
 
 public:
-    TransClipHandler(ScTable& rClipTab, const ScTable& rSrcTab, SCTAB nSrcTab, SCCOL nSrcCol,
-                     size_t nTopRow, size_t nEndRow, SCROW nTransRow, SCROW nRowDestOffset,
-                     bool bAsLink, bool bWasCut, const InsertDeleteFlags& nFlags,
-                     const bool bIncludeFiltered, std::vector<SCROW>& rFilteredRows)
+    TransClipHandler(ScTable& rClipTab, const ScTable& rSrcTab, SCTAB nSrcTab, SCCOL nCol1,
+                     SCCOL nSrcCol, size_t nTopRow, size_t nEndRow, SCROW nCombinedStartRow,
+                     SCROW nRowDestOffset, bool bAsLink, bool bWasCut,
+                     const InsertDeleteFlags& nFlags, const bool bIncludeFiltered,
+                     std::vector<SCROW>& rFilteredRows)
         : mrClipTab(rClipTab)
         , mrSrcTab(rSrcTab)
         , mnSrcTab(nSrcTab)
+        , mnCol1(nCol1)
         , mnSrcCol(nSrcCol)
         , mnTopRow(nTopRow)
         , mnEndRow(nEndRow)
-        , mnTransRow(nTransRow)
+        , mnTransRow(nSrcCol - nCol1 + nCombinedStartRow)
         , mnRowDestOffset(nRowDestOffset)
         , mbAsLink(bAsLink)
         , mbWasCut(bWasCut)
@@ -879,7 +883,7 @@ public:
             return;
         }
 
-        SCCOL nTransCol = nRow - mnTopRow - mnFilteredRows + mnRowDestOffset;
+        SCCOL nTransCol = mnCol1 + nRow - mnTopRow - mnFilteredRows + mnRowDestOffset;
         mrClipTab.SetValue(nTransCol, mnTransRow, fVal);
     }
 
@@ -898,7 +902,7 @@ public:
             return;
         }
 
-        SCCOL nTransCol = nRow - mnTopRow - mnFilteredRows + mnRowDestOffset;
+        SCCOL nTransCol = mnCol1 + nRow - mnTopRow - mnFilteredRows + mnRowDestOffset;
         mrClipTab.SetRawString(nTransCol, mnTransRow, rStr);
     }
 
@@ -917,7 +921,7 @@ public:
             return;
         }
 
-        SCCOL nTransCol = nRow - mnTopRow - mnFilteredRows + mnRowDestOffset;
+        SCCOL nTransCol = mnCol1 + nRow - mnTopRow - mnFilteredRows + mnRowDestOffset;
         mrClipTab.SetEditText(nTransCol, mnTransRow, ScEditUtil::Clone(*p, mrClipTab.GetDoc()));
     }
 
@@ -946,7 +950,7 @@ public:
         if (!mbWasCut)
             pNew->TransposeReference();
 
-        SCCOL nTransCol = nRow - mnTopRow - mnFilteredRows + mnRowDestOffset;
+        SCCOL nTransCol = mnCol1 + nRow - mnTopRow - mnFilteredRows + mnRowDestOffset;
         mrClipTab.SetFormulaCell(nTransCol, mnTransRow, pNew);
     }
 
@@ -974,8 +978,8 @@ public:
 }
 
 void ScTable::TransposeClip(SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2,
-                            SCROW nRowDestOffset, ScTable* pTransClip, InsertDeleteFlags nFlags,
-                            bool bAsLink, bool bIncludeFiltered)
+                            SCROW nCombinedStartRow, SCROW nRowDestOffset, ScTable* pTransClip,
+                            InsertDeleteFlags nFlags, bool bAsLink, bool bIncludeFiltered)
 {
     bool bWasCut = rDocument.IsCutMode();
 
@@ -983,9 +987,9 @@ void ScTable::TransposeClip(SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2,
     {
         std::vector<SCROW> aFilteredRows;
 
-        TransClipHandler aFunc(*pTransClip, *this, nTab, nCol, nRow1, nRow2,
-                               static_cast<SCROW>(nCol - nCol1), nRowDestOffset, bAsLink, bWasCut,
-                               nFlags, bIncludeFiltered, aFilteredRows);
+        TransClipHandler aFunc(*pTransClip, *this, nTab, nCol1, nCol, nRow1, nRow2,
+                               nCombinedStartRow, nRowDestOffset, bAsLink, bWasCut, nFlags,
+                               bIncludeFiltered, aFilteredRows);
 
         const sc::CellStoreType& rCells = aCol[nCol].maCells;
 


More information about the Libreoffice-commits mailing list