sc/inc/SheetView.hxx | 6 +- sc/qa/unit/types/SortOrderReverserTest.cxx | 86 ++++++++++++++++++----------- sc/source/core/data/SheetView.cxx | 37 +++++++----- sc/source/core/data/table3.cxx | 2 sc/source/ui/view/viewfunc.cxx | 6 +- 5 files changed, 85 insertions(+), 52 deletions(-)
New commits: commit 52eb8676e04a196eb8b540faa5d9b88705b4bad1 Author: Tomaž Vajngerl <[email protected]> AuthorDate: Thu Feb 5 18:30:35 2026 +0900 Commit: Miklos Vajna <[email protected]> CommitDate: Fri Feb 13 14:48:59 2026 +0100 sc: Add column range to SortOrderInfo for unsort and resort Column range is needed so we can reject all the cells that are not in the sorted cell range when we try to unsort and resort. This makes checking that we actually are in the sorted range more robust. Change-Id: I0323e720926a32cf2115005bd22207970e40276e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/198736 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sc/inc/SheetView.hxx b/sc/inc/SheetView.hxx index 41f247c162eb..9d5daa3180b7 100644 --- a/sc/inc/SheetView.hxx +++ b/sc/inc/SheetView.hxx @@ -23,6 +23,8 @@ namespace sc /** Contains the information of an performed sort action */ struct SC_DLLPUBLIC SortOrderInfo { + SCCOL mnFirstColumn = -1; + SCCOL mnLastColumn = -1; SCROW mnFirstRow = -1; SCROW mnLastRow = -1; std::vector<SCCOLROW> maOrder; // Remember the sort order @@ -40,8 +42,8 @@ public: * Uses the sort order. The row must be between range of [first row, last row] * or it will return the input row without modification. **/ - SCROW unsort(SCROW nRow) const; - SCROW resort(SCROW nRow) const; + SCROW unsort(SCROW nRow, SCCOL nColumn) const; + SCROW resort(SCROW nRow, SCCOL nColumn) const; /** Adds or combines the order indices. * diff --git a/sc/qa/unit/types/SortOrderReverserTest.cxx b/sc/qa/unit/types/SortOrderReverserTest.cxx index 2cac807f1ea6..532d24857edb 100644 --- a/sc/qa/unit/types/SortOrderReverserTest.cxx +++ b/sc/qa/unit/types/SortOrderReverserTest.cxx @@ -23,34 +23,50 @@ CPPUNIT_TEST_FIXTURE(SortOrderReverserTest, testUnsort) // Unchanged order - output rows should be the same as input { sc::SortOrderReverser aReverser; - aReverser.addOrderIndices({ 5, 8, { 1, 2, 3, 4 }, {} }); + aReverser.addOrderIndices({ 0, 0, 5, 8, { 1, 2, 3, 4 }, {} }); - CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(5)); - CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.unsort(6)); - CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.unsort(7)); - CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(8)); + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(5, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.unsort(6, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.unsort(7, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(8, 0)); } // Reversed order - output rows should be in reverse order than input { sc::SortOrderReverser aReverser; - aReverser.addOrderIndices({ 5, 8, { 4, 3, 2, 1 }, {} }); + aReverser.addOrderIndices({ 0, 0, 5, 8, { 4, 3, 2, 1 }, {} }); - CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(5)); - CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.unsort(6)); - CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.unsort(7)); - CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(8)); + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(5, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.unsort(6, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.unsort(7, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(8, 0)); + + // Column outside sort area - no change + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(5, 1)); + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(8, 1)); + + // Row outside sort area - no change + CPPUNIT_ASSERT_EQUAL(SCROW(4), aReverser.unsort(4, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(9), aReverser.unsort(9, 0)); } // Some random order - output rows should follow the order { sc::SortOrderReverser aReverser; - aReverser.addOrderIndices({ 5, 8, { 3, 1, 4, 2 }, {} }); + aReverser.addOrderIndices({ 0, 0, 5, 8, { 3, 1, 4, 2 }, {} }); + + CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.unsort(5, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(6, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(7, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.unsort(8, 0)); - CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.unsort(5)); - CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(6)); - CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(7)); - CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.unsort(8)); + // Column outside sort area - no change + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(5, 1)); + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(8, 1)); + + // Row outside sort area - no change + CPPUNIT_ASSERT_EQUAL(SCROW(4), aReverser.unsort(4, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(9), aReverser.unsort(9, 0)); } } @@ -59,16 +75,16 @@ CPPUNIT_TEST_FIXTURE(SortOrderReverserTest, testCombiningSortOrder) sc::SortOrderReverser aReverser; // first sorting order - aReverser.addOrderIndices({ 5, 8, { 3, 1, 4, 2 }, {} }); + aReverser.addOrderIndices({ 0, 0, 5, 8, { 3, 1, 4, 2 }, {} }); // second sorting order - only reverses the order - aReverser.addOrderIndices({ 5, 8, { 4, 3, 2, 1 }, {} }); + aReverser.addOrderIndices({ 0, 0, 5, 8, { 4, 3, 2, 1 }, {} }); // check - CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.unsort(5)); - CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(6)); - CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(7)); - CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.unsort(8)); + CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.unsort(5, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(6, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(7, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.unsort(8, 0)); // directly compare the order std::vector<SCCOLROW> aExpectedOrder{ 2, 4, 1, 3 }; @@ -80,22 +96,30 @@ CPPUNIT_TEST_FIXTURE(SortOrderReverserTest, testResort) { { sc::SortOrderReverser aReverser; - aReverser.addOrderIndices({ 5, 8, { 4, 3, 2, 1 }, {} }); + aReverser.addOrderIndices({ 0, 0, 5, 8, { 4, 3, 2, 1 }, {} }); - CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.resort(5)); - CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.resort(6)); - CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.resort(7)); - CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.resort(8)); + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.resort(5, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.resort(6, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.resort(7, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.resort(8, 0)); } { sc::SortOrderReverser aReverser; - aReverser.addOrderIndices({ 5, 8, { 3, 1, 4, 2 }, {} }); + aReverser.addOrderIndices({ 0, 0, 5, 8, { 3, 1, 4, 2 }, {} }); + + CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.resort(5, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.resort(6, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.resort(7, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.resort(8, 0)); + + // Column outside sort area - no change + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.resort(5, 1)); + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.resort(8, 1)); - CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.resort(5)); - CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.resort(6)); - CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.resort(7)); - CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.resort(8)); + // Row outside sort area - no change + CPPUNIT_ASSERT_EQUAL(SCROW(4), aReverser.resort(4, 0)); + CPPUNIT_ASSERT_EQUAL(SCROW(9), aReverser.resort(9, 0)); } } diff --git a/sc/source/core/data/SheetView.cxx b/sc/source/core/data/SheetView.cxx index a5778259252b..70a5177d7401 100644 --- a/sc/source/core/data/SheetView.cxx +++ b/sc/source/core/data/SheetView.cxx @@ -46,33 +46,38 @@ void SortOrderReverser::addOrderIndices(SortOrderInfo const& rSortInfo) } } -SCROW SortOrderReverser::unsort(SCROW nRow) const +SCROW SortOrderReverser::unsort(SCROW nRow, SCCOL nColumn) const { if (maSortInfo.maOrder.empty()) return nRow; - if (nRow >= maSortInfo.mnFirstRow && nRow <= maSortInfo.mnLastRow) - { - size_t index = nRow - maSortInfo.mnFirstRow; - auto nUnsortedRow = maSortInfo.mnFirstRow + maSortInfo.maOrder[index] - 1; - return nUnsortedRow; - } - return nRow; + if (nColumn < maSortInfo.mnFirstColumn || nColumn > maSortInfo.mnLastColumn) + return nRow; + + if (nRow < maSortInfo.mnFirstRow || nRow > maSortInfo.mnLastRow) + return nRow; + + size_t index = nRow - maSortInfo.mnFirstRow; + auto nUnsortedRow = maSortInfo.mnFirstRow + maSortInfo.maOrder[index] - 1; + return nUnsortedRow; } -SCROW SortOrderReverser::resort(SCROW nRow) const +SCROW SortOrderReverser::resort(SCROW nRow, SCCOL nColumn) const { if (maSortInfo.maOrder.empty()) return nRow; - if (nRow >= maSortInfo.mnFirstRow && nRow <= maSortInfo.mnLastRow) + if (nColumn < maSortInfo.mnFirstColumn || nColumn > maSortInfo.mnLastColumn) + return nRow; + + if (nRow < maSortInfo.mnFirstRow || nRow > maSortInfo.mnLastRow) + return nRow; + + SCCOLROW nOrderValue = nRow - maSortInfo.mnFirstRow + 1; + for (size_t nIndex = 0; nIndex < maSortInfo.maOrder.size(); ++nIndex) { - SCCOLROW nOrderValue = nRow - maSortInfo.mnFirstRow + 1; - for (size_t nIndex = 0; nIndex < maSortInfo.maOrder.size(); ++nIndex) - { - if (maSortInfo.maOrder[nIndex] == nOrderValue) - return maSortInfo.mnFirstRow + nIndex; - } + if (maSortInfo.maOrder[nIndex] == nOrderValue) + return maSortInfo.mnFirstRow + nIndex; } return nRow; } diff --git a/sc/source/core/data/table3.cxx b/sc/source/core/data/table3.cxx index cb804219971d..07839f24d148 100644 --- a/sc/source/core/data/table3.cxx +++ b/sc/source/core/data/table3.cxx @@ -1765,6 +1765,8 @@ void ScTable::Sort( aSortOrderInfo.maKeyStates.push_back(rState); } + aSortOrderInfo.mnFirstColumn = rSortParam.nCol1; + aSortOrderInfo.mnLastColumn = rSortParam.nCol2; aSortOrderInfo.mnFirstRow = nRow1; aSortOrderInfo.mnLastRow = nLastRow; aSortOrderInfo.maOrder = pArray->GetOrderIndices(); diff --git a/sc/source/ui/view/viewfunc.cxx b/sc/source/ui/view/viewfunc.cxx index 91cb4f23ce5b..a61de84b1111 100644 --- a/sc/source/ui/view/viewfunc.cxx +++ b/sc/source/ui/view/viewfunc.cxx @@ -853,7 +853,7 @@ void ScViewFunc::EnterData( SCCOL nCol, SCROW nRow, SCTAB nTab, { SCROW nUnsortedRow = nRow; if (pManager->getSortOrder()) - nUnsortedRow = pManager->getSortOrder()->unsort(nUnsortedRow); + nUnsortedRow = pManager->getSortOrder()->unsort(nUnsortedRow, nCol); applyText(*this, nCol, nUnsortedRow, nSheetViewTab, rString, bNumFmtChanged); } else if (GetViewData().GetSheetViewID() == pSheetView->getID()) @@ -874,9 +874,9 @@ void ScViewFunc::EnterData( SCCOL nCol, SCROW nRow, SCTAB nTab, auto pSheetView = pManager->get(nSheetViewID); if (pSheetView->getSortOrder()) - nUnsortedRow = pSheetView->getSortOrder()->unsort(nUnsortedRow); + nUnsortedRow = pSheetView->getSortOrder()->unsort(nUnsortedRow, nCol); if (pManager->getSortOrder()) - nUnsortedRow = pManager->getSortOrder()->resort(nUnsortedRow); + nUnsortedRow = pManager->getSortOrder()->resort(nUnsortedRow, nCol); } applyText(*this, nCol, nUnsortedRow, rTab, rString, bNumFmtChanged); }
