tdf#147611: fix indices

The maKeyState vector in ScSortParam is initialized with three elements,
and they are never removed. The code in ScVbaRange::Sort incorrectly used
1-based indices into that vector.

This was broken since commit a02b445c39d969fedc554fc2c500b88a27a13906
Author: Albert Thuswaldner <albert.thuswaldner@gmail.com>
Date:   Tue Mar 20 19:38:29 2012 +0100

fdo#45747 remove the limitation to 3 sort entries in calc part1

It was fixed in commit 568d3912bf8ced76ecb9506bccc3bd361daba082
Author: Kohei Yoshida <kohei.yoshida@gmail.com>
Date:   Wed Apr 04 15:30:35 2012 -0400

Cleanup. ScPivot(Collection) is no more.

but was restored by commit 3e887edcaacc5b0f5e35d682a259124648e84229
Author: Markus Mohrhard <markus.mohrhard@googlemail.com>
Date:   Thu Apr 5 05:05:40 2012 +0200

Revert "Cleanup. ScPivot(Collection) is no more."

Change-Id: I009252e794c9365f0aef8a61daf9cbd40eca8b75
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130441
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

2 files changed, 67 insertions, 3 deletions

diff --git a/sc/qa/extras/macros-test.cxx b/sc/qa/extras/macros-test.cxx
index cc77943ab879..2516024442b6 100644
--- a/sc/qa/extras/macros-test.cxx
+++ b/sc/qa/extras/macros-test.cxx
@@ -17,6 +17,7 @@
 
 #include <docsh.hxx>
 #include <document.hxx>
+#include <sortparam.hxx>
 
 #include <com/sun/star/sheet/XSpreadsheet.hpp>
 
@@ -65,6 +66,7 @@ public:
     void testTdf146742();
     void testMacroButtonFormControlXlsxExport();
     void testShapeLayerId();
+    void testVbaRangeSort();
 
     CPPUNIT_TEST_SUITE(ScMacrosTest);
     CPPUNIT_TEST(testStarBasic);
@@ -94,6 +96,7 @@ public:
     CPPUNIT_TEST(testTdf146742);
     CPPUNIT_TEST(testMacroButtonFormControlXlsxExport);
     CPPUNIT_TEST(testShapeLayerId);
+    CPPUNIT_TEST(testVbaRangeSort);
 
     CPPUNIT_TEST_SUITE_END();
 };
@@ -1047,6 +1050,67 @@ void ScMacrosTest::testShapeLayerId()
     pDocSh->DoClose();
 }
 
+void ScMacrosTest::testVbaRangeSort()
+{
+    auto xComponent = loadFromDesktop("private:factory/scalc");
+
+    css::uno::Reference<css::document::XEmbeddedScripts> xDocScr(xComponent, UNO_QUERY_THROW);
+    auto xLibs = xDocScr->getBasicLibraries();
+    auto xLibrary = xLibs->createLibrary("TestLibrary");
+    xLibrary->insertByName(
+        "TestModule",
+        uno::Any(OUString("Option VBASupport 1\n"
+                          "Sub TestRangeSort\n"
+                          "  Range(Cells(1, 1), Cells(3, 1)).Select\n"
+                          "  Selection.Sort Key1:=Range(\"A1\"), Header:=False\n"
+                          "End Sub\n")));
+
+    Any aRet;
+    Sequence<sal_Int16> aOutParamIndex;
+    Sequence<Any> aOutParam;
+
+    SfxObjectShell* pFoundShell = SfxObjectShell::GetShellFromComponent(xComponent);
+    ScDocShell* pDocSh = static_cast<ScDocShell*>(pFoundShell);
+    CPPUNIT_ASSERT(pDocSh);
+    ScDocument& rDoc = pDocSh->GetDocument();
+
+    rDoc.SetValue(ScAddress(0, 0, 0), 1.0);
+    rDoc.SetValue(ScAddress(0, 1, 0), 0.5);
+    rDoc.SetValue(ScAddress(0, 2, 0), 2.0);
+
+    // Without the fix in place, this call would have crashed in debug builds with failed assertion
+    ErrCode result = SfxObjectShell::CallXScript(
+        xComponent,
+        "vnd.sun.Star.script:TestLibrary.TestModule.TestRangeSort?language=Basic&location=document",
+        {}, aRet, aOutParamIndex, aOutParam);
+    CPPUNIT_ASSERT_EQUAL(ERRCODE_NONE, result);
+
+    CPPUNIT_ASSERT_EQUAL(0.5, rDoc.GetValue(ScAddress(0, 0, 0)));
+    CPPUNIT_ASSERT_EQUAL(1.0, rDoc.GetValue(ScAddress(0, 1, 0)));
+    CPPUNIT_ASSERT_EQUAL(2.0, rDoc.GetValue(ScAddress(0, 2, 0)));
+
+    // Change sheet's first param sorting order
+    ScSortParam aParam;
+    rDoc.GetSortParam(aParam, 0);
+    aParam.maKeyState[0].bAscending = false;
+    rDoc.SetSortParam(aParam, 0);
+
+    result = SfxObjectShell::CallXScript(
+        xComponent,
+        "vnd.sun.Star.script:TestLibrary.TestModule.TestRangeSort?language=Basic&location=document",
+        {}, aRet, aOutParamIndex, aOutParam);
+    CPPUNIT_ASSERT_EQUAL(ERRCODE_NONE, result);
+
+    // Without the fix in place, this test would have failed in non-debug builds with
+    // - Expected: 2
+    // - Actual  : 0.5
+    CPPUNIT_ASSERT_EQUAL(2.0, rDoc.GetValue(ScAddress(0, 0, 0)));
+    CPPUNIT_ASSERT_EQUAL(1.0, rDoc.GetValue(ScAddress(0, 1, 0)));
+    CPPUNIT_ASSERT_EQUAL(0.5, rDoc.GetValue(ScAddress(0, 2, 0)));
+
+    pDocSh->DoClose();
+}
+
 ScMacrosTest::ScMacrosTest()
       : UnoApiTest("/sc/qa/extras/testdocuments")
 {
diff --git a/sc/source/ui/vba/vbarange.cxx b/sc/source/ui/vba/vbarange.cxx
index d15420bb698c..0c0e5f256e02 100644
--- a/sc/source/ui/vba/vbarange.cxx
+++ b/sc/source/ui/vba/vbarange.cxx
@@ -3413,9 +3413,9 @@ ScVbaRange::Sort( const uno::Any& Key1, const uno::Any& Order1, const uno::Any&
 
     // set up defaults
 
-    sal_Int16 nOrder1 = aSortParam.maKeyState[1].bAscending ? excel::XlSortOrder::xlAscending : excel::XlSortOrder::xlDescending;
-    sal_Int16 nOrder2 = aSortParam.maKeyState[2].bAscending ? excel::XlSortOrder::xlAscending : excel::XlSortOrder::xlDescending;
-    sal_Int16 nOrder3 = aSortParam.maKeyState[3].bAscending ? excel::XlSortOrder::xlAscending : excel::XlSortOrder::xlDescending;
+    sal_Int16 nOrder1 = aSortParam.maKeyState[0].bAscending ? excel::XlSortOrder::xlAscending : excel::XlSortOrder::xlDescending;
+    sal_Int16 nOrder2 = aSortParam.maKeyState[1].bAscending ? excel::XlSortOrder::xlAscending : excel::XlSortOrder::xlDescending;
+    sal_Int16 nOrder3 = aSortParam.maKeyState[2].bAscending ? excel::XlSortOrder::xlAscending : excel::XlSortOrder::xlDescending;
 
     sal_Int16 nCustom = aSortParam.nUserIndex;
     sal_Int16 nSortMethod = excel::XlSortMethod::xlPinYin;