tdf#126678 - Consider "Include formats" option during sort

Change-Id: Ib972ad6c5042bde6b0c79bf10bace6baab1e935e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111234
Tested-by: Jenkins
Reviewed-by: Andreas Heinisch <andreas.heinisch@yahoo.de>

8 files changed, 135 insertions, 30 deletions

diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx
index 4f88f556e9eb..56791cfd5f61 100644
--- a/sc/inc/column.hxx
+++ b/sc/inc/column.hxx
@@ -218,17 +218,19 @@ public:
                 // data only:
     bool        IsEmptyBlock(SCROW nStartRow, SCROW nEndRow) const;
     SCSIZE      GetEmptyLinesInBlock( SCROW nStartRow, SCROW nEndRow, ScDirection eDir ) const;
-    bool        HasDataAt(SCROW nRow, bool bConsiderCellNotes=false,
-                          bool bConsiderCellDrawObjects=false) const;
-    bool        HasDataAt(sc::ColumnBlockConstPosition& rBlockPos, SCROW nRow, bool bConsiderCellNotes=false,
-                          bool bConsiderCellDrawObjects=false) const;
-    bool        HasDataAt(sc::ColumnBlockPosition& rBlockPos, SCROW nRow, bool bConsiderCellNotes=false,
-                          bool bConsiderCellDrawObjects=false);
+    bool        HasDataAt(SCROW nRow, bool bConsiderCellNotes = false,
+                   bool bConsiderCellDrawObjects = false, bool bConsiderCellFormats = false) const;
+    bool        HasDataAt(sc::ColumnBlockConstPosition& rBlockPos, SCROW nRow,
+                   bool bConsiderCellNotes = false, bool bConsiderCellDrawObjects = false,
+                   bool bConsiderCellFormats = false) const;
+    bool        HasDataAt(sc::ColumnBlockPosition& rBlockPos, SCROW nRow, bool bConsiderCellNotes = false,
+                   bool bConsiderCellDrawObjects = false, bool bConsiderCellFormats = false);
     bool        HasVisibleDataAt(SCROW nRow) const;
     SCROW       GetFirstDataPos() const;
     SCROW       GetLastDataPos() const;
-    SCROW       GetLastDataPos( SCROW nLastRow, bool bConsiderCellNotes=false,
-                                bool bConsiderCellDrawObjects=false ) const;
+    SCROW       GetLastDataPos(SCROW nLastRow, bool bConsiderCellNotes = false,
+                         bool bConsiderCellDrawObjects = false,
+                         bool bConsiderCellFormats = false) const;
     bool        GetPrevDataPos(SCROW& rRow) const;
     bool        GetNextDataPos(SCROW& rRow) const;
     bool        TrimEmptyBlocks(SCROW& rRowStart, SCROW& rRowEnd) const;
diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx
index f9582dc4837b..96d7ec940059 100644
--- a/sc/inc/document.hxx
+++ b/sc/inc/document.hxx
@@ -1407,6 +1407,8 @@ public:
                                 If TRUE, consider the presence of cell notes besides data.
                         @param  bConsiderCellDrawObjects
                                 If TRUE, consider the presence of draw objects anchored to the cell.
+                        @param bConsiderCellFormats
+                                If TRUE, consider the presence of cell formats.
 
                         @returns true if there is any data, false if not.
                      */
@@ -1415,7 +1417,8 @@ public:
                                           SCCOL& rEndCol, SCROW& rEndRow, bool bColumnsOnly,
                                           bool bStickyTopRow = false, bool bStickyLeftCol = false,
                                           bool bConsiderCellNotes = false,
-                                          bool bConsiderCellDrawObjects = false ) const;
+                                          bool bConsiderCellDrawObjects = false,
+                                          bool bConsiderCellFormats = false ) const;
 
     /**
      * Return the last non-empty row position in given columns that's no
diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index ddc859715b8e..211e317cb06d 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -589,10 +589,11 @@ public:
     bool        ShrinkToUsedDataArea( bool& o_bShrunk, SCCOL& rStartCol, SCROW& rStartRow,
                                       SCCOL& rEndCol, SCROW& rEndRow, bool bColumnsOnly,
                                       bool bStickyTopRow, bool bStickyLeftCol, bool bConsiderCellNotes,
-                                      bool bConsiderCellDrawObjects ) const;
+                                      bool bConsiderCellDrawObjects, bool bConsiderCellPatterns ) const;
 
-    SCROW GetLastDataRow( SCCOL nCol1, SCCOL nCol2, SCROW nLastRow,
-                         bool bConsiderCellNotes = false, bool bConsiderCellDrawObjects = false ) const;
+    SCROW       GetLastDataRow( SCCOL nCol1, SCCOL nCol2, SCROW nLastRow, bool bConsiderCellNotes = false,
+                                bool bConsiderCellDrawObjects = false,
+                                bool bConsiderCellPatterns = false ) const;
 
     SCSIZE      GetEmptyLinesInBlock( SCCOL nStartCol, SCROW nStartRow,
                                         SCCOL nEndCol, SCROW nEndRow, ScDirection eDir ) const;
diff --git a/sc/qa/uitest/sort/tdf126678.py b/sc/qa/uitest/sort/tdf126678.py
new file mode 100644
index 000000000000..0d63c8c35dd0
--- /dev/null
+++ b/sc/qa/uitest/sort/tdf126678.py
@@ -0,0 +1,75 @@
+# -*- tab-width: 4; indent-tabs-mode: nil; py-indent-offset: 4 -*-
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+from uitest.framework import UITestCase
+from uitest.uihelper.common import get_state_as_dict
+from uitest.uihelper.common import select_pos
+from uitest.uihelper.calc import enter_text_to_cell
+from libreoffice.calc.document import get_cell_by_position
+from libreoffice.uno.propertyvalue import mkPropertyValues
+
+class tdf126678(UITestCase):
+
+    def execute_sort_dialog(self, gridwin, bIncludeFormats):
+        gridwin.executeAction("SELECT", mkPropertyValues({"RANGE": "A1:B3"}))
+
+        self.ui_test.execute_dialog_through_command(".uno:DataSort")
+        xDialog = self.xUITest.getTopFocusWindow()
+        xTabs = xDialog.getChild("tabcontrol")
+        select_pos(xTabs, "1")
+
+        xIncludeFormats = xDialog.getChild("formats")
+
+        if (get_state_as_dict(xIncludeFormats)["Selected"]) != bIncludeFormats:
+            xIncludeFormats.executeAction("CLICK", tuple())
+
+        xOk = xDialog.getChild("ok")
+        self.ui_test.close_dialog_through_button(xOk)
+
+    def test_tdf126678(self):
+        calc_doc = self.ui_test.create_doc_in_start_center("calc")
+        xCalcDoc = self.xUITest.getTopFocusWindow()
+        gridwin = xCalcDoc.getChild("grid_window")
+        document = self.ui_test.get_component()
+
+        enter_text_to_cell(gridwin, "A1", "Text 2")
+        enter_text_to_cell(gridwin, "A2", "Text 3")
+        enter_text_to_cell(gridwin, "A3", "Text 1")
+
+        # Set the background of the corresponding cell
+        colorProperty = mkPropertyValues({"BackgroundColor": 16776960})
+        gridwin.executeAction("SELECT", mkPropertyValues({"CELL": "B2"}))
+        self.xUITest.executeCommandWithParameters(".uno:BackgroundColor", colorProperty)
+
+        self.execute_sort_dialog(gridwin, "false")
+
+        self.assertEqual("Text 1", get_cell_by_position(document, 0, 0, 0).getString())
+        self.assertEqual("Text 2", get_cell_by_position(document, 0, 0, 1).getString())
+        self.assertEqual("Text 3", get_cell_by_position(document, 0, 0, 2).getString())
+
+        # Sorting without option "including formats" does not include cells with cell formats
+        self.assertEqual(get_cell_by_position(document, 0, 1, 0).CellBackColor, -1)
+        self.assertEqual(get_cell_by_position(document, 0, 1, 1).CellBackColor, 16776960)
+        self.assertEqual(get_cell_by_position(document, 0, 1, 2).CellBackColor, -1)
+
+        self.xUITest.executeCommand(".uno:Undo")
+
+        self.execute_sort_dialog(gridwin, "true")
+
+        self.assertEqual("Text 1", get_cell_by_position(document, 0, 0, 0).getString())
+        self.assertEqual("Text 2", get_cell_by_position(document, 0, 0, 1).getString())
+        self.assertEqual("Text 3", get_cell_by_position(document, 0, 0, 2).getString())
+
+        # Sorting with option "including formats" includes all cells with visible cell formats
+        # tdf126678 - Without the fix in place, the test would have failed with
+        # AssertionError: -1 != 16776960
+        self.assertEqual(get_cell_by_position(document, 0, 1, 0).CellBackColor, -1)
+        self.assertEqual(get_cell_by_position(document, 0, 1, 1).CellBackColor, -1)
+        self.assertEqual(get_cell_by_position(document, 0, 1, 2).CellBackColor, 16776960)
+
+        self.ui_test.close_doc()
+
+# vim: set shiftwidth=4 softtabstop=4 expandtab:
diff --git a/sc/source/core/data/column2.cxx b/sc/source/core/data/column2.cxx
index e6430758cc6f..dda91a5cf4f4 100644
--- a/sc/source/core/data/column2.cxx
+++ b/sc/source/core/data/column2.cxx
@@ -1351,7 +1351,7 @@ SCROW ScColumn::GetLastDataPos() const
 }
 
 SCROW ScColumn::GetLastDataPos( SCROW nLastRow, bool bConsiderCellNotes,
-                                bool bConsiderCellDrawObjects ) const
+                                bool bConsiderCellDrawObjects, bool bConsiderCellFormats ) const
 {
     sc::CellStoreType::const_position_type aPos = maCells.position(std::min(nLastRow,GetDoc().MaxRow()));
 
@@ -1361,6 +1361,9 @@ SCROW ScColumn::GetLastDataPos( SCROW nLastRow, bool bConsiderCellNotes,
     if (bConsiderCellDrawObjects && !IsDrawObjectsEmptyBlock(nLastRow, nLastRow))
         return nLastRow;
 
+    if (bConsiderCellFormats && HasVisibleAttrIn(nLastRow, nLastRow))
+        return nLastRow;
+
     if (aPos.first->type != sc::element_type_empty)
         return nLastRow;
 
@@ -3128,7 +3131,8 @@ void ScColumn::FindDataAreaPos(SCROW& rRow, bool bDown) const
     rRow = nLastRow;
 }
 
-bool ScColumn::HasDataAt(SCROW nRow, bool bConsiderCellNotes, bool bConsiderCellDrawObjects) const
+bool ScColumn::HasDataAt(SCROW nRow, bool bConsiderCellNotes, bool bConsiderCellDrawObjects,
+                         bool bConsiderCellFormats) const
 {
     if (bConsiderCellNotes && !IsNotesEmptyBlock(nRow, nRow))
         return true;
@@ -3136,11 +3140,15 @@ bool ScColumn::HasDataAt(SCROW nRow, bool bConsiderCellNotes, bool bConsiderCell
     if (bConsiderCellDrawObjects && !IsDrawObjectsEmptyBlock(nRow, nRow))
         return true;
 
+    if (bConsiderCellFormats && HasVisibleAttrIn(nRow, nRow))
+        return true;
+
     return maCells.get_type(nRow) != sc::element_type_empty;
 }
 
 bool ScColumn::HasDataAt(sc::ColumnBlockConstPosition& rBlockPos, SCROW nRow,
-                         bool bConsiderCellNotes, bool bConsiderCellDrawObjects) const
+                         bool bConsiderCellNotes, bool bConsiderCellDrawObjects,
+                         bool bConsiderCellFormats) const
 {
     if (bConsiderCellNotes && !IsNotesEmptyBlock(nRow, nRow))
         return true;
@@ -3148,6 +3156,9 @@ bool ScColumn::HasDataAt(sc::ColumnBlockConstPosition& rBlockPos, SCROW nRow,
     if (bConsiderCellDrawObjects && !IsDrawObjectsEmptyBlock(nRow, nRow))
         return true;
 
+    if (bConsiderCellFormats && HasVisibleAttrIn(nRow, nRow))
+        return true;
+
     std::pair<sc::CellStoreType::const_iterator,size_t> aPos = maCells.position(rBlockPos.miCellPos, nRow);
     if (aPos.first == maCells.end())
         return false;
@@ -3156,7 +3167,7 @@ bool ScColumn::HasDataAt(sc::ColumnBlockConstPosition& rBlockPos, SCROW nRow,
 }
 
 bool ScColumn::HasDataAt(sc::ColumnBlockPosition& rBlockPos, SCROW nRow,
-                         bool bConsiderCellNotes, bool bConsiderCellDrawObjects)
+                         bool bConsiderCellNotes, bool bConsiderCellDrawObjects, bool bConsiderCellFormats)
 {
     if (bConsiderCellNotes && !IsNotesEmptyBlock(nRow, nRow))
         return true;
@@ -3164,6 +3175,9 @@ bool ScColumn::HasDataAt(sc::ColumnBlockPosition& rBlockPos, SCROW nRow,
     if (bConsiderCellDrawObjects && !IsDrawObjectsEmptyBlock(nRow, nRow))
         return true;
 
+    if (bConsiderCellFormats && HasVisibleAttrIn(nRow, nRow))
+        return true;
+
     std::pair<sc::CellStoreType::iterator,size_t> aPos = maCells.position(rBlockPos.miCellPos, nRow);
     if (aPos.first == maCells.end())
         return false;
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index 9755bd74c90e..ad18c2f077bf 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -1062,15 +1062,16 @@ bool ScDocument::ShrinkToDataArea(SCTAB nTab, SCCOL& rStartCol, SCROW& rStartRow
 bool ScDocument::ShrinkToUsedDataArea( bool& o_bShrunk, SCTAB nTab, SCCOL& rStartCol,
         SCROW& rStartRow, SCCOL& rEndCol, SCROW& rEndRow, bool bColumnsOnly,
         bool bStickyTopRow, bool bStickyLeftCol, bool bConsiderCellNotes,
-        bool bConsiderCellDrawObjects ) const
+        bool bConsiderCellDrawObjects, bool bConsiderCellFormats ) const
 {
     if (!ValidTab(nTab) || nTab >= static_cast<SCTAB> (maTabs.size()) || !maTabs[nTab])
     {
         o_bShrunk = false;
         return false;
     }
-    return maTabs[nTab]->ShrinkToUsedDataArea( o_bShrunk, rStartCol, rStartRow, rEndCol, rEndRow,
-            bColumnsOnly, bStickyTopRow, bStickyLeftCol, bConsiderCellNotes, bConsiderCellDrawObjects );
+    return maTabs[nTab]->ShrinkToUsedDataArea(
+        o_bShrunk, rStartCol, rStartRow, rEndCol, rEndRow, bColumnsOnly, bStickyTopRow,
+        bStickyLeftCol, bConsiderCellNotes, bConsiderCellDrawObjects, bConsiderCellFormats);
 }
 
 SCROW ScDocument::GetLastDataRow( SCTAB nTab, SCCOL nCol1, SCCOL nCol2, SCROW nLastRow ) const
diff --git a/sc/source/core/data/table1.cxx b/sc/source/core/data/table1.cxx
index 79a33320358c..b70e5b490cde 100644
--- a/sc/source/core/data/table1.cxx
+++ b/sc/source/core/data/table1.cxx
@@ -976,7 +976,7 @@ bool ScTable::GetDataAreaSubrange( ScRange& rRange ) const
 
 bool ScTable::ShrinkToUsedDataArea( bool& o_bShrunk, SCCOL& rStartCol, SCROW& rStartRow,
         SCCOL& rEndCol, SCROW& rEndRow, bool bColumnsOnly, bool bStickyTopRow, bool bStickyLeftCol,
-        bool bConsiderCellNotes, bool bConsiderCellDrawObjects ) const
+        bool bConsiderCellNotes, bool bConsiderCellDrawObjects, bool bConsiderCellFormats ) const
 {
     rStartCol = std::min<SCCOL>( rStartCol, aCol.size()-1 );
     // check for rEndCol is done below.
@@ -1016,6 +1016,9 @@ bool ScTable::ShrinkToUsedDataArea( bool& o_bShrunk, SCCOL& rStartCol, SCROW& rS
             if (bConsiderCellDrawObjects && !aCol[rEndCol].IsDrawObjectsEmptyBlock( rStartRow, rEndRow ))
                 break;
 
+            if (bConsiderCellFormats && aCol[rEndCol].HasVisibleAttrIn(rStartRow, rEndRow))
+                break;
+
             --rEndCol;
             o_bShrunk = true;
         }
@@ -1035,6 +1038,9 @@ bool ScTable::ShrinkToUsedDataArea( bool& o_bShrunk, SCCOL& rStartCol, SCROW& rS
                 if (bConsiderCellDrawObjects && !aCol[rStartCol].IsDrawObjectsEmptyBlock( rStartRow, rEndRow ))
                     break;
 
+                if (bConsiderCellFormats && aCol[rEndCol].HasVisibleAttrIn(rStartRow, rEndRow))
+                    break;
+
                 ++rStartCol;
                 o_bShrunk = true;
             }
@@ -1052,7 +1058,8 @@ bool ScTable::ShrinkToUsedDataArea( bool& o_bShrunk, SCCOL& rStartCol, SCROW& rS
                 bool bFound = false;
                 for (SCCOL i=rStartCol; i<=rEndCol && !bFound; i++)
                 {
-                    if (aCol[i].HasDataAt( rStartRow, bConsiderCellNotes, bConsiderCellDrawObjects))
+                    if (aCol[i].HasDataAt(rStartRow, bConsiderCellNotes, bConsiderCellDrawObjects,
+                                          bConsiderCellFormats))
                         bFound = true;
                 }
                 if (!bFound)
@@ -1067,8 +1074,8 @@ bool ScTable::ShrinkToUsedDataArea( bool& o_bShrunk, SCCOL& rStartCol, SCROW& rS
 
         while (rStartRow < rEndRow)
         {
-            SCROW nLastDataRow = GetLastDataRow( rStartCol, rEndCol, rEndRow,
-                                                 bConsiderCellNotes, bConsiderCellDrawObjects);
+            SCROW nLastDataRow = GetLastDataRow(rStartCol, rEndCol, rEndRow, bConsiderCellNotes,
+                                                bConsiderCellDrawObjects, bConsiderCellFormats);
             if (0 <= nLastDataRow && nLastDataRow < rEndRow)
             {
                 rEndRow = std::max( rStartRow, nLastDataRow);
@@ -1082,11 +1089,11 @@ bool ScTable::ShrinkToUsedDataArea( bool& o_bShrunk, SCCOL& rStartCol, SCROW& rS
     return rStartCol != rEndCol || (bColumnsOnly ?
             !aCol[rStartCol].IsEmptyBlock( rStartRow, rEndRow) :
             (rStartRow != rEndRow ||
-                aCol[rStartCol].HasDataAt( rStartRow, bConsiderCellNotes, bConsiderCellDrawObjects)));
+                aCol[rStartCol].HasDataAt( rStartRow, bConsiderCellNotes, bConsiderCellDrawObjects, bConsiderCellFormats )));
 }
 
-SCROW ScTable::GetLastDataRow( SCCOL nCol1, SCCOL nCol2, SCROW nLastRow,
-                               bool bConsiderCellNotes, bool bConsiderCellDrawObjects ) const
+SCROW ScTable::GetLastDataRow( SCCOL nCol1, SCCOL nCol2, SCROW nLastRow, bool bConsiderCellNotes,
+                               bool bConsiderCellDrawObjects, bool bConsiderCellFormats ) const
 {
     if ( !IsColValid( nCol1 ) || !ValidCol( nCol2 ) )
         return -1;
@@ -1096,7 +1103,8 @@ SCROW ScTable::GetLastDataRow( SCCOL nCol1, SCCOL nCol2, SCROW nLastRow,
     SCROW nNewLastRow = 0;
     for (SCCOL i = nCol1; i <= nCol2; ++i)
     {
-        SCROW nThis = aCol[i].GetLastDataPos(nLastRow, bConsiderCellNotes, bConsiderCellDrawObjects);
+        SCROW nThis = aCol[i].GetLastDataPos(nLastRow, bConsiderCellNotes, bConsiderCellDrawObjects,
+                                             bConsiderCellFormats);
         if (nNewLastRow < nThis)
             nNewLastRow = nThis;
     }
diff --git a/sc/source/ui/docshell/dbdocfun.cxx b/sc/source/ui/docshell/dbdocfun.cxx
index 9ed3e0f41ff1..a02d26bb0659 100644
--- a/sc/source/ui/docshell/dbdocfun.cxx
+++ b/sc/source/ui/docshell/dbdocfun.cxx
@@ -526,9 +526,10 @@ bool ScDBDocFunc::Sort( SCTAB nTab, const ScSortParam& rSortParam,
     // column (depending on direction) in any case, not just if it has headers,
     // so empty leading cells will be sorted to the end.
     bool bShrunk = false;
-    rDoc.ShrinkToUsedDataArea( bShrunk, nTab, aLocalParam.nCol1, aLocalParam.nRow1,
-            aLocalParam.nCol2, aLocalParam.nRow2, false, aLocalParam.bByRow, !aLocalParam.bByRow,
-            aLocalParam.bIncludeComments, aLocalParam.bIncludeGraphicObjects );
+    rDoc.ShrinkToUsedDataArea(bShrunk, nTab, aLocalParam.nCol1, aLocalParam.nRow1,
+                              aLocalParam.nCol2, aLocalParam.nRow2, false, aLocalParam.bByRow,
+                              !aLocalParam.bByRow, aLocalParam.bIncludeComments,
+                              aLocalParam.bIncludeGraphicObjects, aLocalParam.bIncludePattern);
 
     SCROW nStartRow = aLocalParam.nRow1;
     if (aLocalParam.bByRow && aLocalParam.bHasHeader && nStartRow < aLocalParam.nRow2)