Resolves: tdf#122232 Move TabStartCol logic to ScTable::GetNextPos()

... instead of blindly applying it to every move if set.

Change-Id: Ief2efb4eb2288cd479852d5a250c2523715de38b
Reviewed-on: https://gerrit.libreoffice.org/82513
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins
(cherry picked from commit 25160a56f4fd8a18c27eaa77b0539ab267b80294)
Reviewed-on: https://gerrit.libreoffice.org/82536
Tested-by: Xisco Faulí <xiscofauli@libreoffice.org>
Reviewed-by: Xisco Faulí <xiscofauli@libreoffice.org>

7 files changed, 72 insertions, 50 deletions

diff --git a/sc/inc/address.hxx b/sc/inc/address.hxx
index b7baaa4c0bc0..724b41e4249a 100644
--- a/sc/inc/address.hxx
+++ b/sc/inc/address.hxx
@@ -90,6 +90,7 @@ const SCTAB TABLEID_DOC       = SCTAB_MAX;  // entire document, e.g. protect
 const SCROW SCROWS32K         = 32000;
 const SCCOL SCCOL_REPEAT_NONE = SCCOL_MAX;
 const SCROW SCROW_REPEAT_NONE = SCROW_MAX;
+const SCCOL SC_TABSTART_NONE  = SCCOL_MAX;
 
 #define MAXROW_30   8191
 
diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx
index af372c270675..ed7b5e2acccb 100644
--- a/sc/inc/document.hxx
+++ b/sc/inc/document.hxx
@@ -1444,7 +1444,8 @@ public:
 
     void               FindAreaPos( SCCOL& rCol, SCROW& rRow, SCTAB nTab, ScMoveDirection eDirection ) const;
     SC_DLLPUBLIC void  GetNextPos( SCCOL& rCol, SCROW& rRow, SCTAB nTab, SCCOL nMovX, SCROW nMovY,
-                                   bool bMarked, bool bUnprotected, const ScMarkData& rMark ) const;
+                                   bool bMarked, bool bUnprotected, const ScMarkData& rMark,
+                                   SCCOL nTabStartCol = SC_TABSTART_NONE ) const;
 
     bool               GetNextMarkedCell( SCCOL& rCol, SCROW& rRow, SCTAB nTab,
                                           const ScMarkData& rMark );
diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index 29a7b28fe635..01d809013815 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -588,7 +588,7 @@ public:
 
     void        FindAreaPos( SCCOL& rCol, SCROW& rRow, ScMoveDirection eDirection ) const;
     void        GetNextPos( SCCOL& rCol, SCROW& rRow, SCCOL nMovX, SCROW nMovY,
-                                bool bMarked, bool bUnprotected, const ScMarkData& rMark ) const;
+                            bool bMarked, bool bUnprotected, const ScMarkData& rMark, SCCOL nTabStartCol ) const;
 
     bool        SkipRow( const SCCOL rCol, SCROW& rRow, const SCROW nMovY, const ScMarkData& rMark,
                          const bool bUp, const SCROW nUsedY, const bool bMarked, const bool bSheetProtected ) const;
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index 3a65ae04aad7..9b9ca5e83661 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -6075,7 +6075,7 @@ void ScDocument::FindAreaPos( SCCOL& rCol, SCROW& rRow, SCTAB nTab, ScMoveDirect
 }
 
 void ScDocument::GetNextPos( SCCOL& rCol, SCROW& rRow, SCTAB nTab, SCCOL nMovX, SCROW nMovY,
-                                bool bMarked, bool bUnprotected, const ScMarkData& rMark ) const
+        bool bMarked, bool bUnprotected, const ScMarkData& rMark, SCCOL nTabStartCol ) const
 {
     OSL_ENSURE( !nMovX || !nMovY, "GetNextPos: only X or Y" );
 
@@ -6084,7 +6084,7 @@ void ScDocument::GetNextPos( SCCOL& rCol, SCROW& rRow, SCTAB nTab, SCCOL nMovX,
     aCopyMark.MarkToMulti();
 
     if (ValidTab(nTab) && nTab < static_cast<SCTAB>(maTabs.size()) && maTabs[nTab])
-        maTabs[nTab]->GetNextPos( rCol, rRow, nMovX, nMovY, bMarked, bUnprotected, aCopyMark );
+        maTabs[nTab]->GetNextPos( rCol, rRow, nMovX, nMovY, bMarked, bUnprotected, aCopyMark, nTabStartCol );
 }
 
 //  Data operations
diff --git a/sc/source/core/data/table1.cxx b/sc/source/core/data/table1.cxx
index b899bbb91cbf..49eb0b52b384 100644
--- a/sc/source/core/data/table1.cxx
+++ b/sc/source/core/data/table1.cxx
@@ -1386,7 +1386,7 @@ bool ScTable::SkipRow( const SCCOL nCol, SCROW& rRow, const SCROW nMovY,
 }
 
 void ScTable::GetNextPos( SCCOL& rCol, SCROW& rRow, SCCOL nMovX, SCROW nMovY,
-                                bool bMarked, bool bUnprotected, const ScMarkData& rMark ) const
+        bool bMarked, bool bUnprotected, const ScMarkData& rMark, SCCOL nTabStartCol ) const
 {
     // Ensure bMarked is set only if there is a mark.
     assert( !bMarked || rMark.IsMarked() || rMark.IsMultiMarked());
@@ -1447,53 +1447,82 @@ void ScTable::GetNextPos( SCCOL& rCol, SCROW& rRow, SCCOL nMovX, SCROW nMovY,
             rCol = nCol;
             rRow = nRow;
         }
+
+        // Caller ensures actually moving nMovY to jump to prev/next row's
+        // start col.
+        if (nTabStartCol != SC_TABSTART_NONE)
+            rCol = nTabStartCol;
+
         return;
     }
 
     if ( nMovY && (bMarked || bUnprotected))
     {
-        bool bUp = (nMovY < 0);
-        const SCCOL nColAdd = (bUp ? -1 : 1);
-        sal_uInt16 nWrap = 0;
-
-        if (bMarked)
-            nRow = rMark.GetNextMarked( nCol, nRow, bUp );
-
-        while ( SkipRow( nCol, nRow, nMovY, rMark, bUp, nEndRow, bMarked, bSheetProtected ))
-            ;
-
-        while ( nRow < nStartRow || nRow > nEndRow )
+        do
         {
-            nCol += nColAdd;
+            const bool bUp = (nMovY < 0);
+            const SCCOL nColAdd = (bUp ? -1 : 1);
 
-            while (nStartCol <= nCol && nCol <= nEndCol && ValidCol(nCol) && ColHidden(nCol))
-                nCol += nColAdd;    //  skip hidden cols
+            if (bMarked)
+                nRow = rMark.GetNextMarked( nCol, nRow, bUp );
 
-            if (nCol < nStartCol)
+            if (nTabStartCol != SC_TABSTART_NONE)
             {
-                nCol = nEndCol;
-
-                if (++nWrap >= 2)
-                    return;
+                /* NOTE: If current rCol < nTabStartCol when going down, there
+                 * is no way to detect if the previous Tab wrapped around to
+                 * the next row or if it was a Shift+Tab going backwards. The
+                 * result after a wrap is an odd jump to the next row's
+                 * nTabStartCol, which is logical though and always has been
+                 * the case. Similar for rCol > nTabStartCol when going up.
+                 * Related, it would be nice to limit advancing the position
+                 * within bounds even if another wrap would occur, but again we
+                 * can't tell if previously Tab or Shift+Tab was used, so we
+                 * don't know if it would be nTabStartCol to nEndCol (for Tab)
+                 * or nStartCol to nTabStartCol (for Shift+Tab). */
+
+                // Continue moving horizontally.
+                nMovX = nColAdd;
+                nCol = nTabStartCol;
+                break;  // do
             }
-            else if (nCol > nEndCol)
+
+            while ( SkipRow( nCol, nRow, nMovY, rMark, bUp, nEndRow, bMarked, bSheetProtected ))
+                ;
+
+            sal_uInt16 nWrap = 0;
+            while ( nRow < nStartRow || nRow > nEndRow )
             {
-                nCol = nStartCol;
+                nCol += nColAdd;
 
-                if (++nWrap >= 2)
-                    return;
-            }
-            if (nRow < nStartRow)
-                nRow = nEndRow;
-            else if (nRow > nEndRow)
-                nRow = nStartRow;
+                while (nStartCol <= nCol && nCol <= nEndCol && ValidCol(nCol) && ColHidden(nCol))
+                    nCol += nColAdd;    //  skip hidden cols
 
-            if (bMarked)
-                nRow = rMark.GetNextMarked( nCol, nRow, bUp );
+                if (nCol < nStartCol)
+                {
+                    nCol = nEndCol;
 
-            while ( SkipRow( nCol, nRow, nMovY, rMark, bUp, nEndRow, bMarked, bSheetProtected ))
-                ;
-        }
+                    if (++nWrap >= 2)
+                        return;
+                }
+                else if (nCol > nEndCol)
+                {
+                    nCol = nStartCol;
+
+                    if (++nWrap >= 2)
+                        return;
+                }
+                if (nRow < nStartRow)
+                    nRow = nEndRow;
+                else if (nRow > nEndRow)
+                    nRow = nStartRow;
+
+                if (bMarked)
+                    nRow = rMark.GetNextMarked( nCol, nRow, bUp );
+
+                while ( SkipRow( nCol, nRow, nMovY, rMark, bUp, nEndRow, bMarked, bSheetProtected ))
+                    ;
+            }
+        } while (false);
     }
 
     if ( nMovX && ( bMarked || bUnprotected ) )
diff --git a/sc/source/ui/inc/viewdata.hxx b/sc/source/ui/inc/viewdata.hxx
index 8f65e80c9a47..4e3494873d23 100644
--- a/sc/source/ui/inc/viewdata.hxx
+++ b/sc/source/ui/inc/viewdata.hxx
@@ -30,7 +30,6 @@
 #include <o3tl/typed_flags_set.hxx>
 
 #define SC_SIZE_NONE        65535
-const SCCOL SC_TABSTART_NONE = SCCOL_MAX;
 
 enum class ScFillMode
 {
diff --git a/sc/source/ui/view/tabview3.cxx b/sc/source/ui/view/tabview3.cxx
index bfdc208bf128..b63cd1d6b595 100644
--- a/sc/source/ui/view/tabview3.cxx
+++ b/sc/source/ui/view/tabview3.cxx
@@ -1442,17 +1442,9 @@ void ScTabView::MoveCursorEnter( bool bShift )          // bShift -> up/down
     }
     else
     {
-        pDoc->GetNextPos( nNewX, nNewY, nTab, nMoveX, nMoveY, false, true, rMark );
-
-        if ( nMoveY != 0 && !nMoveX )
-        {
-            // after Tab and Enter back to the starting column again
-            SCCOL nTabCol = aViewData.GetTabStartCol();
-            if (nTabCol != SC_TABSTART_NONE)
-            {
-                nNewX = nTabCol;
-            }
-        }
+        // After Tab and Enter back to the starting column again.
+        const SCCOL nTabStartCol = ((nMoveY != 0 && !nMoveX) ? aViewData.GetTabStartCol() : SC_TABSTART_NONE);
+        pDoc->GetNextPos( nNewX, nNewY, nTab, nMoveX, nMoveY, false, true, rMark, nTabStartCol );
 
         MoveCursorRel( nNewX - nCurX, nNewY - nCurY, SC_FOLLOW_LINE, false);
     }