Turns out ScHorizontalIterator was still broken. Fix it for real.

And a unit test to accompany the fix.

Change-Id: I41e9451049d3c6ab7b3fd7904bcef3675979884c

3 files changed, 74 insertions, 22 deletions

diff --git a/sc/qa/unit/ucalc.cxx b/sc/qa/unit/ucalc.cxx
index 1122b60ef7d3..55d5c2a8388a 100644
--- a/sc/qa/unit/ucalc.cxx
+++ b/sc/qa/unit/ucalc.cxx
@@ -53,6 +53,7 @@
 #include "globstr.hrc"
 #include "tokenarray.hxx"
 #include "scopetools.hxx"
+#include "dociter.hxx"
 
 #include "formula/IFunctionDescription.hxx"
 
@@ -131,6 +132,8 @@ public:
     void testSheetsFunc();
     void testVolatileFunc();
 
+    void testHorizontalIterator();
+
     /**
      * Basic test for formula dependency tracking.
      */
@@ -306,6 +309,7 @@ public:
     CPPUNIT_TEST(testCopyToDocument);
     CPPUNIT_TEST(testSheetsFunc);
     CPPUNIT_TEST(testVolatileFunc);
+    CPPUNIT_TEST(testHorizontalIterator);
     CPPUNIT_TEST(testFormulaDepTracking);
     CPPUNIT_TEST(testFormulaDepTracking2);
     CPPUNIT_TEST(testCellBroadcaster);
@@ -1778,6 +1782,53 @@ void Test::testVolatileFunc()
     m_pDoc->DeleteTab(0);
 }
 
+void Test::testHorizontalIterator()
+{
+    m_pDoc->InsertTab(0, "test");
+
+    // Raw data
+    const char* aData[][2] = {
+        { "A", "B" },
+        { "C", "1" },
+        { "D", "2" },
+        { "E", "3" }
+    };
+
+    ScAddress aPos(0,0,0);
+    insertRangeData(m_pDoc, aPos, aData, SAL_N_ELEMENTS(aData));
+    ScHorizontalCellIterator aIter(m_pDoc, 0, 0, 0, 1, SAL_N_ELEMENTS(aData));
+
+    struct {
+        SCCOL nCol;
+        SCROW nRow;
+        const char* pVal;
+    } aChecks[] = {
+        { 0, 0, "A" },
+        { 1, 0, "B" },
+        { 0, 1, "C" },
+        { 1, 1, "1" },
+        { 0, 2, "D" },
+        { 1, 2, "2" },
+        { 0, 3, "E" },
+        { 1, 3, "3" },
+    };
+
+    SCCOL nCol;
+    SCROW nRow;
+    size_t i = 0, n = SAL_N_ELEMENTS(aChecks);
+    for (ScRefCellValue* pCell = aIter.GetNext(nCol, nRow); pCell; pCell = aIter.GetNext(nCol, nRow), ++i)
+    {
+        if (i >= n)
+            CPPUNIT_FAIL("Iterator claims there is more data than there should be.");
+
+        CPPUNIT_ASSERT_EQUAL(aChecks[i].nCol, nCol);
+        CPPUNIT_ASSERT_EQUAL(aChecks[i].nRow, nRow);
+        CPPUNIT_ASSERT_EQUAL(OUString::createFromAscii(aChecks[i].pVal), pCell->getString());
+    }
+
+    m_pDoc->DeleteTab(0);
+}
+
 void Test::testFormulaDepTracking()
 {
     CPPUNIT_ASSERT_MESSAGE ("failed to insert sheet", m_pDoc->InsertTab (0, "foo"));
@@ -6785,13 +6836,9 @@ void Test::testAnchoredRotatedShape()
         ScDrawLayer::SetCellAnchoredFromPosition(*pObj, *m_pDoc, 0);
 
         Rectangle aSnap = pObj->GetSnapRect();
-        printf("expected height %ld actual %ld\n", aRotRect.GetHeight(), aSnap.GetHeight() );
         CPPUNIT_ASSERT_EQUAL( true, testEqualsWithTolerance( aRotRect.GetHeight(), aSnap.GetHeight(), TOLERANCE ) );
-        printf("expected width %ld actual %ld\n", aRotRect.GetWidth(), aSnap.GetWidth() );
         CPPUNIT_ASSERT_EQUAL( true, testEqualsWithTolerance( aRotRect.GetWidth(), aSnap.GetWidth(), TOLERANCE ) );
-        printf("expected left %ld actual %ld\n", aRotRect.Left(), aSnap.Left() );
         CPPUNIT_ASSERT_EQUAL( true, testEqualsWithTolerance( aRotRect.Left(), aSnap.Left(), TOLERANCE ) );
-        printf("expected right %ld actual %ld\n", aRotRect.Top(), aSnap.Top() );
         CPPUNIT_ASSERT_EQUAL( true, testEqualsWithTolerance( aRotRect.Top(), aSnap.Top(), TOLERANCE ) );
 
         ScDrawObjData aAnchor;
@@ -6814,20 +6861,14 @@ void Test::testAnchoredRotatedShape()
 
         aSnap = pObj->GetSnapRect();
 
-        printf("expected new height %ld actual %ld\n", aRotRect.GetHeight(), aSnap.GetHeight() );
         // ensure that width and height have been adjusted accordingly
         CPPUNIT_ASSERT_EQUAL( true, testEqualsWithTolerance( aRotRect.GetHeight(), aSnap.GetHeight(), TOLERANCE ) );
-        printf("expected new width %ld %ld\n", aRotRect.GetWidth(), aSnap.GetWidth() );
         CPPUNIT_ASSERT_EQUAL( true, testEqualsWithTolerance( aRotRect.GetWidth(), aSnap.GetWidth(), TOLERANCE ) );
 
         // ensure that anchor start and end addresses haven't changed
-        printf("expected startrow %ld actual %ld\n", (long)aAnchor.maStart.Row(), (long)pData->maStart.Row()  );
         CPPUNIT_ASSERT_EQUAL( aAnchor.maStart.Row(), pData->maStart.Row() ); // start row 0
-        printf("expected startcol %ld actual %ld\n", (long)aAnchor.maStart.Col(), (long)pData->maStart.Col()  );
         CPPUNIT_ASSERT_EQUAL( aAnchor.maStart.Col(), pData->maStart.Col() ); // start column 5
-        printf("expected endrow %ld actual %ld\n", (long)aAnchor.maEnd.Row(), (long)pData->maEnd.Row()  );
         CPPUNIT_ASSERT_EQUAL( aAnchor.maEnd.Row(), pData->maEnd.Row() ); // end row 3
-        printf("expected endcol %ld actual %ld\n", (long)aAnchor.maEnd.Col(), (long)pData->maEnd.Col()  );
         CPPUNIT_ASSERT_EQUAL( aAnchor.maEnd.Col(), pData->maEnd.Col() ); // end col 7
     }
     m_pDoc->DeleteTab(0);
diff --git a/sc/source/core/data/cellvalue.cxx b/sc/source/core/data/cellvalue.cxx
index 617eb298f265..8e6283605f9a 100644
--- a/sc/source/core/data/cellvalue.cxx
+++ b/sc/source/core/data/cellvalue.cxx
@@ -583,6 +583,8 @@ OUString ScRefCellValue::getString()
 {
     switch (meType)
     {
+        case CELLTYPE_VALUE:
+            return OUString::number(mfValue);
         case CELLTYPE_STRING:
             return *mpString;
         case CELLTYPE_EDIT:
diff --git a/sc/source/core/data/dociter.cxx b/sc/source/core/data/dociter.cxx
index b384ca2d7896..fb1becda12c2 100644
--- a/sc/source/core/data/dociter.cxx
+++ b/sc/source/core/data/dociter.cxx
@@ -1742,6 +1742,24 @@ bool ScHorizontalCellIterator::GetPos( SCCOL& rCol, SCROW& rRow )
     return bMore;
 }
 
+namespace {
+
+bool advanceBlock(size_t nRow, sc::CellStoreType::const_iterator& rPos, const sc::CellStoreType::const_iterator& rEnd)
+{
+    // This block is behind the current row position. Advance the block.
+    for (++rPos; rPos != rEnd; ++rPos)
+    {
+        if (nRow < rPos->position + rPos->size)
+            // Found the block that contains the specified row.
+            return true;
+    }
+
+    // No more blocks.
+    return false;
+}
+
+}
+
 void ScHorizontalCellIterator::Advance()
 {
     // Find the next non-empty cell in the current row.
@@ -1759,7 +1777,8 @@ void ScHorizontalCellIterator::Advance()
             continue;
 
         if (r.maPos->position + r.maPos->size <= nRow)
-            continue;
+            if (!advanceBlock(nRow, r.maPos, r.maEnd))
+                continue;
 
         // Found in the current row.
         mnCol = i;
@@ -1793,18 +1812,8 @@ void ScHorizontalCellIterator::Advance()
             }
 
             if (r.maPos->position + r.maPos->size <= nRow)
-            {
-                // This block is behind the current row position. Advance the block.
-                for (++r.maPos; r.maPos != r.maEnd; ++r.maPos)
-                {
-                    if (nRow < r.maPos->position + r.maPos->size)
-                        break;
-                }
-
-                if (r.maPos == r.maEnd)
-                    // This column has ended.
+                if (!advanceBlock(nRow, r.maPos, r.maEnd))
                     continue;
-            }
 
             if (r.maPos->type == sc::element_type_empty)
             {